Discussion:
[PATCH weston v5 01/14] compositor-drm: Add backend pointer to drm_output
Daniel Stone
2018-07-20 19:03:22 UTC
Permalink
Add this for convenience, so it's easier to access when we add the DRM
backend debug scope.

Signed-off-by: Daniel Stone <***@collabora.com>
---
libweston/compositor-drm.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 704ac32c7..e27671437 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -463,6 +463,7 @@ struct drm_head {

struct drm_output {
struct weston_output base;
+ struct drm_backend *backend;

uint32_t crtc_id; /* object ID to pass to DRM functions */
int pipe; /* index of CRTC in resource array / bitmasks */
@@ -6101,6 +6102,8 @@ drm_output_create(struct weston_compositor *compositor, const char *name)
if (output == NULL)
return NULL;

+ output->backend = b;
+
weston_output_init(&output->base, compositor, name);

output->base.enable = drm_output_enable;
--
2.17.1
Daniel Stone
2018-07-20 19:03:24 UTC
Permalink
From: Pekka Paalanen <***@iki.fi>

weston_debug is both a libweston API for relaying debugging messages,
and the compositor-debug wayland protocol implementation for accessing those
debug messages from a Wayland client.

weston_debug_compositor_{create,destroy}() are private API, hence not
exported.

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

append the debug scope name along with the timestamp in
weston_debug_scope_timestamp API

Signed-off-by: Maniraj Devadoss <***@in.bosch.com>
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>

Add explicit advertisement of debug scope names.

Signed-off-by: Daniel Stone <***@collabora.com>
---
Makefile.am | 2 +
libweston/compositor.c | 6 +
libweston/compositor.h | 9 +
libweston/weston-debug.c | 723 +++++++++++++++++++++++++++++++++++++++
libweston/weston-debug.h | 107 ++++++
5 files changed, 847 insertions(+)
create mode 100644 libweston/weston-debug.c
create mode 100644 libweston/weston-debug.h

diff --git a/Makefile.am b/Makefile.am
index 66eb365c5..c2d9048b3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -98,6 +98,8 @@ ***@LIBWESTON_MAJOR@_la_SOURCES = \
libweston/linux-dmabuf.h \
libweston/pixel-formats.c \
libweston/pixel-formats.h \
+ libweston/weston-debug.c \
+ libweston/weston-debug.h \
shared/helpers.h \
shared/matrix.c \
shared/matrix.h \
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 9deb7817f..8768f67a0 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -6361,6 +6361,9 @@ weston_compositor_create(struct wl_display *display, void *user_data)
ec, bind_presentation))
goto fail;

+ if (weston_debug_compositor_create(ec) < 0)
+ goto fail;
+
if (weston_input_init(ec) != 0)
goto fail;

@@ -6699,9 +6702,12 @@ weston_compositor_destroy(struct weston_compositor *compositor)

weston_plugin_api_destroy_list(compositor);

+
if (compositor->heads_changed_source)
wl_event_source_remove(compositor->heads_changed_source);

+ weston_debug_compositor_destroy(compositor);
+
free(compositor);
}

diff --git a/libweston/compositor.h b/libweston/compositor.h
index fd0ff7b5b..83448b70f 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1049,6 +1049,7 @@ struct weston_touch_calibrator;

struct weston_desktop_xwayland;
struct weston_desktop_xwayland_interface;
+struct weston_debug_compositor;

struct weston_compositor {
struct wl_signal destroy_signal;
@@ -1161,6 +1162,8 @@ struct weston_compositor {
weston_touch_calibration_save_func touch_calibration_save;
struct weston_layer calibrator_layer;
struct weston_touch_calibrator *touch_calibrator;
+
+ struct weston_debug_compositor *weston_debug;
};

struct weston_buffer {
@@ -2315,6 +2318,12 @@ int
weston_compositor_enable_touch_calibrator(struct weston_compositor *compositor,
weston_touch_calibration_save_func save);

+int
+weston_debug_compositor_create(struct weston_compositor *compositor);
+
+void
+weston_debug_compositor_destroy(struct weston_compositor *compositor);
+
#ifdef __cplusplus
}
#endif
diff --git a/libweston/weston-debug.c b/libweston/weston-debug.c
new file mode 100644
index 000000000..039247f14
--- /dev/null
+++ b/libweston/weston-debug.c
@@ -0,0 +1,723 @@
+/*
+ * Copyright © 2017 Pekka Paalanen <***@iki.fi>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include "weston-debug.h"
+#include "helpers.h"
+#include "compositor.h"
+
+#include "weston-debug-server-protocol.h"
+
+#include <unistd.h>
+#include <stdarg.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/time.h>
+
+/** Main weston-debug context
+ *
+ * One per weston_compositor.
+ *
+ * \internal
+ */
+struct weston_debug_compositor {
+ struct weston_compositor *compositor;
+ struct wl_listener compositor_destroy_listener;
+ struct wl_global *global;
+ struct wl_list scope_list; /**< weston_debug_scope::compositor_link */
+
+ struct weston_debug_scope *list;
+};
+
+/** weston-debug message scope
+ *
+ * This is used for scoping debugging messages. Clients can subscribe to
+ * only the scopes they are interested in. A scope is identified by its name
+ * (also referred to as debug stream name).
+ */
+struct weston_debug_scope {
+ char *name;
+ char *desc;
+ weston_debug_scope_cb begin_cb;
+ void *user_data;
+ struct wl_list stream_list; /**< weston_debug_stream::scope_link */
+ struct wl_list compositor_link;
+};
+
+/** A debug stream created by a client
+ *
+ * A client provides a file descriptor for the server to write debug
+ * messages into. A weston_debug_stream is associated to one
+ * weston_debug_scope via the scope name, and the scope provides the messages.
+ * There can be several streams for the same scope, all streams getting the
+ * same messages.
+ */
+struct weston_debug_stream {
+ int fd; /**< client provided fd */
+ struct wl_resource *resource; /**< weston_debug_stream_v1 object */
+ struct wl_list scope_link;
+};
+
+static struct weston_debug_scope *
+get_scope(struct weston_debug_compositor *wdc, const char *name)
+{
+ struct weston_debug_scope *scope;
+
+ wl_list_for_each(scope, &wdc->scope_list, compositor_link)
+ if (strcmp(name, scope->name) == 0)
+ return scope;
+
+ return NULL;
+}
+
+static void
+stream_close_unlink(struct weston_debug_stream *stream)
+{
+ if (stream->fd != -1)
+ close(stream->fd);
+ stream->fd = -1;
+
+ wl_list_remove(&stream->scope_link);
+ wl_list_init(&stream->scope_link);
+}
+
+static void WL_PRINTF(2, 3)
+stream_close_on_failure(struct weston_debug_stream *stream,
+ const char *fmt, ...)
+{
+ char *msg;
+ va_list ap;
+ int ret;
+
+ stream_close_unlink(stream);
+
+ va_start(ap, fmt);
+ ret = vasprintf(&msg, fmt, ap);
+ va_end(ap);
+
+ if (ret > 0) {
+ weston_debug_stream_v1_send_failure(stream->resource, msg);
+ free(msg);
+ } else {
+ weston_debug_stream_v1_send_failure(stream->resource,
+ "MEMFAIL");
+ }
+}
+
+static struct weston_debug_stream *
+stream_create(struct weston_debug_compositor *wdc, const char *name,
+ int32_t streamfd, struct wl_resource *stream_resource)
+{
+ struct weston_debug_stream *stream;
+ struct weston_debug_scope *scope;
+
+ stream = zalloc(sizeof *stream);
+ if (!stream)
+ return NULL;
+
+ stream->fd = streamfd;
+ stream->resource = stream_resource;
+
+ scope = get_scope(wdc, name);
+ if (scope) {
+ wl_list_insert(&scope->stream_list, &stream->scope_link);
+
+ if (scope->begin_cb)
+ scope->begin_cb(stream, scope->user_data);
+ } else {
+ wl_list_init(&stream->scope_link);
+ stream_close_on_failure(stream,
+ "Debug stream name '%s' is unknown.",
+ name);
+ }
+
+ return stream;
+}
+
+static void
+stream_destroy(struct wl_resource *stream_resource)
+{
+ struct weston_debug_stream *stream;
+
+ stream = wl_resource_get_user_data(stream_resource);
+
+ if (stream->fd != -1)
+ close(stream->fd);
+ wl_list_remove(&stream->scope_link);
+ free(stream);
+}
+
+static void
+weston_debug_stream_destroy(struct wl_client *client,
+ struct wl_resource *stream_resource)
+{
+ wl_resource_destroy(stream_resource);
+}
+
+static const struct weston_debug_stream_v1_interface
+ weston_debug_stream_impl = {
+ weston_debug_stream_destroy
+};
+
+static void
+weston_debug_destroy(struct wl_client *client,
+ struct wl_resource *global_resource)
+{
+ wl_resource_destroy(global_resource);
+}
+
+static void
+weston_debug_subscribe(struct wl_client *client,
+ struct wl_resource *global_resource,
+ const char *name,
+ int32_t streamfd,
+ uint32_t new_stream_id)
+{
+ struct weston_debug_compositor *wdc;
+ struct wl_resource *stream_resource;
+ uint32_t version;
+ struct weston_debug_stream *stream;
+
+ wdc = wl_resource_get_user_data(global_resource);
+ version = wl_resource_get_version(global_resource);
+
+ stream_resource = wl_resource_create(client,
+ &weston_debug_stream_v1_interface,
+ version, new_stream_id);
+ if (!stream_resource)
+ goto fail;
+
+ stream = stream_create(wdc, name, streamfd, stream_resource);
+ if (!stream)
+ goto fail;
+
+ wl_resource_set_implementation(stream_resource,
+ &weston_debug_stream_impl,
+ stream, stream_destroy);
+ return;
+
+fail:
+ close(streamfd);
+ wl_client_post_no_memory(client);
+}
+
+static const struct weston_debug_v1_interface weston_debug_impl = {
+ weston_debug_destroy,
+ weston_debug_subscribe
+};
+
+static void
+bind_weston_debug(struct wl_client *client,
+ void *data, uint32_t version, uint32_t id)
+{
+ struct weston_debug_compositor *wdc = data;
+ struct weston_debug_scope *scope;
+ struct wl_resource *resource;
+
+ resource = wl_resource_create(client,
+ &weston_debug_v1_interface,
+ version, id);
+ if (!resource) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+ wl_resource_set_implementation(resource, &weston_debug_impl,
+ wdc, NULL);
+
+ wl_list_for_each(scope, &wdc->scope_list, compositor_link)
+ weston_debug_v1_send_available(resource, scope->name);
+}
+
+/** Initialize weston-debug structure
+ *
+ * \param compositor The libweston compositor.
+ * \return 0 on success, -1 on failure.
+ *
+ * weston_debug_compositor is a singleton for each weston_compositor.
+ *
+ * Sets weston_compositor::weston_debug.
+ *
+ * \internal
+ */
+int
+weston_debug_compositor_create(struct weston_compositor *compositor)
+{
+ struct weston_debug_compositor *wdc;
+
+ if (compositor->weston_debug)
+ return -1;
+
+ wdc = zalloc(sizeof *wdc);
+ if (!wdc)
+ return -1;
+
+ wdc->compositor = compositor;
+ wl_list_init(&wdc->scope_list);
+
+ compositor->weston_debug = wdc;
+
+ return 0;
+}
+
+/** Destroy weston_debug_compositor structure
+ *
+ * \param compositor The libweston compositor whose weston-debug to tear down.
+ *
+ * Clears weston_compositor::weston_debug.
+ *
+ * \internal
+ */
+void
+weston_debug_compositor_destroy(struct weston_compositor *compositor)
+{
+ struct weston_debug_compositor *wdc = compositor->weston_debug;
+ struct weston_debug_scope *scope;
+
+ if (wdc->global)
+ wl_global_destroy(wdc->global);
+
+ wl_list_for_each(scope, &wdc->scope_list, compositor_link)
+ weston_log("Internal warning: debug scope '%s' has not been destroyed.\n",
+ scope->name);
+
+ /* Remove head to not crash if scope removed later. */
+ wl_list_remove(&wdc->scope_list);
+
+ free(wdc);
+
+ compositor->weston_debug = NULL;
+}
+
+/** Enable weston-debug protocol extension
+ *
+ * \param compositor The libweston compositor where to enable.
+ *
+ * This enables the weston_debug_v1 Wayland protocol extension which any
+ * client can use to get debug messsages from the compositor.
+ *
+ * WARNING: This feature should not be used in production. If a client
+ * provides a file descriptor that blocks writes, it will block the whole
+ * compositor indefinitely.
+ *
+ * There is no control on which client is allowed to subscribe to debug
+ * messages. Any and all clients are allowed.
+ *
+ * The debug extension is disabled by default, and once enabled, cannot be
+ * disabled again.
+ */
+WL_EXPORT void
+weston_compositor_enable_debug_protocol(struct weston_compositor *compositor)
+{
+ struct weston_debug_compositor *wdc;
+
+ wdc = compositor->weston_debug;
+ if (!wdc)
+ return;
+
+ if (wdc->global)
+ return;
+
+ wdc->global = wl_global_create(compositor->wl_display,
+ &weston_debug_v1_interface, 1,
+ wdc, bind_weston_debug);
+ if (!wdc->global)
+ return;
+
+ weston_log("WARNING: debug protocol has been enabled. "
+ "This is a potential denial-of-service attack vector and "
+ "information leak.\n");
+}
+
+/** Register a new debug stream name, creating a debug scope
+ *
+ * \param compositor The libweston compositor where to add.
+ * \param name The debug stream/scope name.
+ * \param desc The debug scope description for humans.
+ * \param begin_cb Optional callback when a client subscribes to this scope.
+ * \param user_data Optional user data pointer for the callback.
+ * \return A valid pointer on success, NULL on failure.
+ *
+ * This function is used to create a debug scope. All debug message printing
+ * happens for a scope, which allows clients to subscribe to the kind of
+ * debug messages they want by \c name.
+ *
+ * \c name must be unique in the \c weston_compositor instance. \c name and
+ * \c description must both be provided. The description is printed when a
+ * client asks for a list of supported debug scopes.
+ *
+ * \c begin_cb, if not NULL, is called when a client subscribes to the
+ * debug scope creating a debug stream. This is for debug scopes that need
+ * to print messages as a response to a client appearing, e.g. printing a
+ * list of windows on demand or a static preamble. The argument \c user_data
+ * is passed in to the callback and is otherwise unused.
+ *
+ * For one-shot debug streams, \c begin_cb should finally call
+ * weston_debug_stream_complete() to close the stream and tell the client
+ * the printing is complete. Otherwise the client expects more to be written
+ * to its file descriptor.
+ *
+ * The debug scope must be destroyed before destroying the
+ * \c weston_compositor.
+ *
+ * \memberof weston_debug_scope
+ * \sa weston_debug_stream, weston_debug_scope_cb
+ */
+WL_EXPORT struct weston_debug_scope *
+weston_compositor_add_debug_scope(struct weston_compositor *compositor,
+ const char *name,
+ const char *description,
+ weston_debug_scope_cb begin_cb,
+ void *user_data)
+{
+ struct weston_debug_compositor *wdc;
+ struct weston_debug_scope *scope;
+
+ if (!compositor || !name || !description) {
+ weston_log("Error: cannot add a debug scope without name or description.\n");
+ return NULL;
+ }
+
+ wdc = compositor->weston_debug;
+ if (!wdc) {
+ weston_log("Error: cannot add debug scope '%s', infra not initialized.\n",
+ name);
+ return NULL;
+ }
+
+ if (get_scope(wdc, name)){
+ weston_log("Error: debug scope named '%s' is already registered.\n",
+ name);
+ return NULL;
+ }
+
+ scope = zalloc(sizeof *scope);
+ if (!scope) {
+ weston_log("Error adding debug scope '%s': out of memory.\n",
+ name);
+ return NULL;
+ }
+
+ scope->name = strdup(name);
+ scope->desc = strdup(description);
+ scope->begin_cb = begin_cb;
+ scope->user_data = user_data;
+ wl_list_init(&scope->stream_list);
+
+ if (!scope->name || !scope->desc) {
+ weston_log("Error adding debug scope '%s': out of memory.\n",
+ name);
+ free(scope->name);
+ free(scope->desc);
+ free(scope);
+ return NULL;
+ }
+
+ wl_list_insert(wdc->scope_list.prev, &scope->compositor_link);
+
+ return scope;
+}
+
+/** Destroy a debug scope
+ *
+ * \param scope The debug scope to destroy.
+ *
+ * Destroys the debug scope, closing all open streams subscribed to it and
+ * sending them each a \c weston_debug_stream_v1.failure event.
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT void
+weston_debug_scope_destroy(struct weston_debug_scope *scope)
+{
+ struct weston_debug_stream *stream;
+
+ if (!scope)
+ return;
+
+ while (!wl_list_empty(&scope->stream_list)) {
+ stream = wl_container_of(scope->stream_list.prev,
+ stream, scope_link);
+
+ stream_close_on_failure(stream, "debug name removed");
+ }
+
+ wl_list_remove(&scope->compositor_link);
+ free(scope->name);
+ free(scope->desc);
+ free(scope);
+}
+
+/** Are there any active subscriptions to the scope?
+ *
+ * \param scope The debug scope to check.
+ * \return True if any streams are open for this scope, false otherwise.
+ *
+ * As printing some debugging messages may be relatively expensive, one
+ * can use this function to determine if there is a need to gather the
+ * debugging information at all. If this function returns false, all
+ * printing for this scope is dropped, so gathering the information is
+ * pointless.
+ *
+ * The return value of this function should not be stored, as new clients
+ * may subscribe to the debug scope later.
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT bool
+weston_debug_scope_is_enabled(struct weston_debug_scope *scope)
+{
+ if (!scope)
+ return false;
+
+ return !wl_list_empty(&scope->stream_list);
+}
+
+/** Write data into a specific debug stream
+ *
+ * \param stream The debug stream to write into.
+ * \param data[in] Pointer to the data to write.
+ * \param len Number of bytes to write.
+ *
+ * Writes the given data (binary verbatim) into the debug stream.
+ * If \c len is zero or negative, the write is silently dropped.
+ *
+ * Writing is continued until all data has been written or
+ * a write fails. If the write fails due to a signal, it is re-tried.
+ * Otherwise on failure, the stream is closed and
+ * \c weston_debug_stream_v1.failure event is sent to the client.
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_write(struct weston_debug_stream *stream,
+ const char *data, size_t len)
+{
+ ssize_t len_ = len;
+ ssize_t ret;
+ int e;
+
+ if (stream->fd == -1)
+ return;
+
+ while (len_ > 0) {
+ ret = write(stream->fd, data, len_);
+ e = errno;
+ if (ret < 0) {
+ if (e == EINTR)
+ continue;
+
+ stream_close_on_failure(stream,
+ "Error writing %zd bytes: %s (%d)",
+ len_, strerror(e), e);
+ break;
+ }
+
+ len_ -= ret;
+ data += ret;
+ }
+}
+
+/** Write a formatted string into a specific debug stream (varargs)
+ *
+ * \param stream The debug stream to write into.
+ * \param fmt Printf-style format string.
+ * \param ap Formatting arguments.
+ *
+ * The behavioral details are the same as for weston_debug_stream_write().
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_vprintf(struct weston_debug_stream *stream,
+ const char *fmt, va_list ap)
+{
+ char *str;
+ int len;
+
+ len = vasprintf(&str, fmt, ap);
+ if (len >= 0) {
+ weston_debug_stream_write(stream, str, len);
+ free(str);
+ } else {
+ stream_close_on_failure(stream, "Out of memory");
+ }
+}
+
+/** Write a formatted string into a specific debug stream
+ *
+ * \param stream The debug stream to write into.
+ * \param fmt Printf-style format string and arguments.
+ *
+ * The behavioral details are the same as for weston_debug_stream_write().
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_printf(struct weston_debug_stream *stream,
+ const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ weston_debug_stream_vprintf(stream, fmt, ap);
+ va_end(ap);
+}
+
+/** Close the debug stream and send success event
+ *
+ * \param stream The debug stream to close.
+ *
+ * Closes the debug stream and sends \c weston_debug_stream_v1.complete
+ * event to the client. This tells the client the debug information dump
+ * is complete.
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_complete(struct weston_debug_stream *stream)
+{
+ stream_close_unlink(stream);
+ weston_debug_stream_v1_send_complete(stream->resource);
+}
+
+/** Write debug data for a scope
+ *
+ * \param scope The debug scope to write for.
+ * \param data[in] Pointer to the data to write.
+ * \param len Number of bytes to write.
+ *
+ * Writes the given data to all subscribed clients' streams.
+ *
+ * The behavioral details for each stream are the same as for
+ * weston_debug_stream_write().
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT void
+weston_debug_scope_write(struct weston_debug_scope *scope,
+ const char *data, size_t len)
+{
+ struct weston_debug_stream *stream;
+
+ if (!scope)
+ return;
+
+ wl_list_for_each(stream, &scope->stream_list, scope_link)
+ weston_debug_stream_write(stream, data, len);
+}
+
+/** Write a formatted string for a scope (varargs)
+ *
+ * \param scope The debug scope to write for.
+ * \param fmt Printf-style format string.
+ * \param ap Formatting arguments.
+ *
+ * Writes to formatted string to all subscribed clients' streams.
+ *
+ * The behavioral details for each stream are the same as for
+ * weston_debug_stream_write().
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT void
+weston_debug_scope_vprintf(struct weston_debug_scope *scope,
+ const char *fmt, va_list ap)
+{
+ static const char oom[] = "Out of memory";
+ char *str;
+ int len;
+
+ if (!weston_debug_scope_is_enabled(scope))
+ return;
+
+ len = vasprintf(&str, fmt, ap);
+ if (len >= 0) {
+ weston_debug_scope_write(scope, str, len);
+ free(str);
+ } else {
+ weston_debug_scope_write(scope, oom, sizeof oom - 1);
+ }
+}
+
+/** Write a formatted string for a scope
+ *
+ * \param scope The debug scope to write for.
+ * \param fmt Printf-style format string and arguments.
+ *
+ * Writes to formatted string to all subscribed clients' streams.
+ *
+ * The behavioral details for each stream are the same as for
+ * weston_debug_stream_write().
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT void
+weston_debug_scope_printf(struct weston_debug_scope *scope,
+ const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ weston_debug_scope_vprintf(scope, fmt, ap);
+ va_end(ap);
+}
+
+/** Format current time as a string
+ * and append the debug scope name to it
+ *
+ * \param scope[in] debug scope.
+ * \param buf[out] Buffer to store the string.
+ * \param len Available size in the buffer in bytes.
+ * \return \c buf
+ *
+ * Reads the current local wall-clock time and formats it into a string.
+ * and append the debug scope name to it.
+ * The string is nul-terminated, even if truncated.
+ */
+WL_EXPORT char *
+weston_debug_scope_timestamp(struct weston_debug_scope *scope,
+ char *buf, size_t len)
+{
+ struct timeval tv;
+ struct tm *bdt;
+ char string[128];
+ size_t ret = 0;
+
+ gettimeofday(&tv, NULL);
+
+ bdt = localtime(&tv.tv_sec);
+ if (bdt)
+ ret = strftime(string, sizeof string,
+ "%Y-%m-%d %H:%M:%S", bdt);
+
+ if (ret > 0)
+ snprintf(buf, len, "[%s.%03ld][%s]", string,
+ tv.tv_usec / 1000, scope->name);
+ else
+ snprintf(buf, len, "[?][%s]", scope->name);
+
+ return buf;
+}
diff --git a/libweston/weston-debug.h b/libweston/weston-debug.h
new file mode 100644
index 000000000..c76cec852
--- /dev/null
+++ b/libweston/weston-debug.h
@@ -0,0 +1,107 @@
+/*
+ * Copyright © 2017 Pekka Paalanen <***@iki.fi>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef WESTON_DEBUG_H
+#define WESTON_DEBUG_H
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdarg.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct weston_compositor;
+
+void
+weston_compositor_enable_debug_protocol(struct weston_compositor *);
+
+struct weston_debug_scope;
+struct weston_debug_stream;
+
+/** weston_debug_scope callback
+ *
+ * \param stream The debug stream.
+ * \param user_data The \c user_data argument given to
+ * weston_compositor_add_debug_scope()
+ *
+ * \memberof weston_debug_scope
+ * \sa weston_debug_stream
+ */
+typedef void (*weston_debug_scope_cb)(struct weston_debug_stream *stream,
+ void *user_data);
+
+struct weston_debug_scope *
+weston_compositor_add_debug_scope(struct weston_compositor *compositor,
+ const char *name,
+ const char *description,
+ weston_debug_scope_cb begin_cb,
+ void *user_data);
+
+void
+weston_debug_scope_destroy(struct weston_debug_scope *scope);
+
+bool
+weston_debug_scope_is_enabled(struct weston_debug_scope *scope);
+
+void
+weston_debug_scope_write(struct weston_debug_scope *scope,
+ const char *data, size_t len);
+
+void
+weston_debug_scope_vprintf(struct weston_debug_scope *scope,
+ const char *fmt, va_list ap);
+
+void
+weston_debug_scope_printf(struct weston_debug_scope *scope,
+ const char *fmt, ...)
+ __attribute__ ((format (printf, 2, 3)));
+
+void
+weston_debug_stream_write(struct weston_debug_stream *stream,
+ const char *data, size_t len);
+
+void
+weston_debug_stream_vprintf(struct weston_debug_stream *stream,
+ const char *fmt, va_list ap);
+
+void
+weston_debug_stream_printf(struct weston_debug_stream *stream,
+ const char *fmt, ...)
+ __attribute__ ((format (printf, 2, 3)));
+
+void
+weston_debug_stream_complete(struct weston_debug_stream *stream);
+
+char *
+weston_debug_scope_timestamp(struct weston_debug_scope *scope,
+ char *buf, size_t len);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* WESTON_DEBUG_H */
--
2.17.1
Ucan, Emre (ADITG/ESB)
2018-07-26 14:25:28 UTC
Permalink
Reviewed-by: Emre Ucan <***@de.adit-jv.com>

Best regards

Emre Ucan
Engineering Software Base (ADITG/ESB)

Tel. +49 5121 49 6937
-----Original Message-----
From: wayland-devel [mailto:wayland-devel-
Sent: Freitag, 20. Juli 2018 21:06
Subject: [PATCH weston v5 03/14] libweston: add weston_debug API and
implementation
weston_debug is both a libweston API for relaying debugging messages,
and the compositor-debug wayland protocol implementation for accessing those
debug messages from a Wayland client.
weston_debug_compositor_{create,destroy}() are private API, hence not
exported.
append the debug scope name along with the timestamp in
weston_debug_scope_timestamp API
Add explicit advertisement of debug scope names.
---
Makefile.am | 2 +
libweston/compositor.c | 6 +
libweston/compositor.h | 9 +
libweston/weston-debug.c | 723
+++++++++++++++++++++++++++++++++++++++
libweston/weston-debug.h | 107 ++++++
5 files changed, 847 insertions(+)
create mode 100644 libweston/weston-debug.c
create mode 100644 libweston/weston-debug.h
diff --git a/Makefile.am b/Makefile.am
index 66eb365c5..c2d9048b3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -98,6 +98,8 @@ ***@LIBWESTON_MAJOR@_la_SOURCES =
\
libweston/linux-dmabuf.h \
libweston/pixel-formats.c \
libweston/pixel-formats.h \
+ libweston/weston-debug.c \
+ libweston/weston-debug.h \
shared/helpers.h \
shared/matrix.c \
shared/matrix.h \
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 9deb7817f..8768f67a0 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -6361,6 +6361,9 @@ weston_compositor_create(struct wl_display *display, void *user_data)
ec, bind_presentation))
goto fail;
+ if (weston_debug_compositor_create(ec) < 0)
+ goto fail;
+
if (weston_input_init(ec) != 0)
goto fail;
@@ -6699,9 +6702,12 @@ weston_compositor_destroy(struct
weston_compositor *compositor)
weston_plugin_api_destroy_list(compositor);
+
if (compositor->heads_changed_source)
wl_event_source_remove(compositor-
Post by Daniel Stone
heads_changed_source);
+ weston_debug_compositor_destroy(compositor);
+
free(compositor);
}
diff --git a/libweston/compositor.h b/libweston/compositor.h
index fd0ff7b5b..83448b70f 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1049,6 +1049,7 @@ struct weston_touch_calibrator;
struct weston_desktop_xwayland;
struct weston_desktop_xwayland_interface;
+struct weston_debug_compositor;
struct weston_compositor {
struct wl_signal destroy_signal;
@@ -1161,6 +1162,8 @@ struct weston_compositor {
weston_touch_calibration_save_func touch_calibration_save;
struct weston_layer calibrator_layer;
struct weston_touch_calibrator *touch_calibrator;
+
+ struct weston_debug_compositor *weston_debug;
};
struct weston_buffer {
@@ -2315,6 +2318,12 @@ int
weston_compositor_enable_touch_calibrator(struct weston_compositor *compositor,
weston_touch_calibration_save_func save);
+int
+weston_debug_compositor_create(struct weston_compositor
*compositor);
+
+void
+weston_debug_compositor_destroy(struct weston_compositor
*compositor);
+
#ifdef __cplusplus
}
#endif
diff --git a/libweston/weston-debug.c b/libweston/weston-debug.c
new file mode 100644
index 000000000..039247f14
--- /dev/null
+++ b/libweston/weston-debug.c
@@ -0,0 +1,723 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include "weston-debug.h"
+#include "helpers.h"
+#include "compositor.h"
+
+#include "weston-debug-server-protocol.h"
+
+#include <unistd.h>
+#include <stdarg.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/time.h>
+
+/** Main weston-debug context
+ *
+ * One per weston_compositor.
+ *
+ * \internal
+ */
+struct weston_debug_compositor {
+ struct weston_compositor *compositor;
+ struct wl_listener compositor_destroy_listener;
+ struct wl_global *global;
+ struct wl_list scope_list; /**<
weston_debug_scope::compositor_link */
+
+ struct weston_debug_scope *list;
+};
+
+/** weston-debug message scope
+ *
+ * This is used for scoping debugging messages. Clients can subscribe to
+ * only the scopes they are interested in. A scope is identified by its name
+ * (also referred to as debug stream name).
+ */
+struct weston_debug_scope {
+ char *name;
+ char *desc;
+ weston_debug_scope_cb begin_cb;
+ void *user_data;
+ struct wl_list stream_list; /**< weston_debug_stream::scope_link */
+ struct wl_list compositor_link;
+};
+
+/** A debug stream created by a client
+ *
+ * A client provides a file descriptor for the server to write debug
+ * messages into. A weston_debug_stream is associated to one
+ * weston_debug_scope via the scope name, and the scope provides the messages.
+ * There can be several streams for the same scope, all streams getting the
+ * same messages.
+ */
+struct weston_debug_stream {
+ int fd; /**< client provided fd */
+ struct wl_resource *resource; /**< weston_debug_stream_v1
object */
+ struct wl_list scope_link;
+};
+
+static struct weston_debug_scope *
+get_scope(struct weston_debug_compositor *wdc, const char *name)
+{
+ struct weston_debug_scope *scope;
+
+ wl_list_for_each(scope, &wdc->scope_list, compositor_link)
+ if (strcmp(name, scope->name) == 0)
+ return scope;
+
+ return NULL;
+}
+
+static void
+stream_close_unlink(struct weston_debug_stream *stream)
+{
+ if (stream->fd != -1)
+ close(stream->fd);
+ stream->fd = -1;
+
+ wl_list_remove(&stream->scope_link);
+ wl_list_init(&stream->scope_link);
+}
+
+static void WL_PRINTF(2, 3)
+stream_close_on_failure(struct weston_debug_stream *stream,
+ const char *fmt, ...)
+{
+ char *msg;
+ va_list ap;
+ int ret;
+
+ stream_close_unlink(stream);
+
+ va_start(ap, fmt);
+ ret = vasprintf(&msg, fmt, ap);
+ va_end(ap);
+
+ if (ret > 0) {
+ weston_debug_stream_v1_send_failure(stream->resource,
msg);
+ free(msg);
+ } else {
+ weston_debug_stream_v1_send_failure(stream->resource,
+ "MEMFAIL");
+ }
+}
+
+static struct weston_debug_stream *
+stream_create(struct weston_debug_compositor *wdc, const char *name,
+ int32_t streamfd, struct wl_resource *stream_resource)
+{
+ struct weston_debug_stream *stream;
+ struct weston_debug_scope *scope;
+
+ stream = zalloc(sizeof *stream);
+ if (!stream)
+ return NULL;
+
+ stream->fd = streamfd;
+ stream->resource = stream_resource;
+
+ scope = get_scope(wdc, name);
+ if (scope) {
+ wl_list_insert(&scope->stream_list, &stream->scope_link);
+
+ if (scope->begin_cb)
+ scope->begin_cb(stream, scope->user_data);
+ } else {
+ wl_list_init(&stream->scope_link);
+ stream_close_on_failure(stream,
+ "Debug stream name '%s' is
unknown.",
+ name);
+ }
+
+ return stream;
+}
+
+static void
+stream_destroy(struct wl_resource *stream_resource)
+{
+ struct weston_debug_stream *stream;
+
+ stream = wl_resource_get_user_data(stream_resource);
+
+ if (stream->fd != -1)
+ close(stream->fd);
+ wl_list_remove(&stream->scope_link);
+ free(stream);
+}
+
+static void
+weston_debug_stream_destroy(struct wl_client *client,
+ struct wl_resource *stream_resource)
+{
+ wl_resource_destroy(stream_resource);
+}
+
+static const struct weston_debug_stream_v1_interface
+ weston_debug_stream_impl
= {
+ weston_debug_stream_destroy
+};
+
+static void
+weston_debug_destroy(struct wl_client *client,
+ struct wl_resource *global_resource)
+{
+ wl_resource_destroy(global_resource);
+}
+
+static void
+weston_debug_subscribe(struct wl_client *client,
+ struct wl_resource *global_resource,
+ const char *name,
+ int32_t streamfd,
+ uint32_t new_stream_id)
+{
+ struct weston_debug_compositor *wdc;
+ struct wl_resource *stream_resource;
+ uint32_t version;
+ struct weston_debug_stream *stream;
+
+ wdc = wl_resource_get_user_data(global_resource);
+ version = wl_resource_get_version(global_resource);
+
+ stream_resource = wl_resource_create(client,
+
&weston_debug_stream_v1_interface,
+ version, new_stream_id);
+ if (!stream_resource)
+ goto fail;
+
+ stream = stream_create(wdc, name, streamfd, stream_resource);
+ if (!stream)
+ goto fail;
+
+ wl_resource_set_implementation(stream_resource,
+ &weston_debug_stream_impl,
+ stream, stream_destroy);
+ return;
+
+ close(streamfd);
+ wl_client_post_no_memory(client);
+}
+
+static const struct weston_debug_v1_interface weston_debug_impl = {
+ weston_debug_destroy,
+ weston_debug_subscribe
+};
+
+static void
+bind_weston_debug(struct wl_client *client,
+ void *data, uint32_t version, uint32_t id)
+{
+ struct weston_debug_compositor *wdc = data;
+ struct weston_debug_scope *scope;
+ struct wl_resource *resource;
+
+ resource = wl_resource_create(client,
+ &weston_debug_v1_interface,
+ version, id);
+ if (!resource) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+ wl_resource_set_implementation(resource, &weston_debug_impl,
+ wdc, NULL);
+
+ wl_list_for_each(scope, &wdc->scope_list, compositor_link)
+ weston_debug_v1_send_available(resource, scope->name);
+}
+
+/** Initialize weston-debug structure
+ *
+ * \param compositor The libweston compositor.
+ * \return 0 on success, -1 on failure.
+ *
+ * weston_debug_compositor is a singleton for each weston_compositor.
+ *
+ * Sets weston_compositor::weston_debug.
+ *
+ * \internal
+ */
+int
+weston_debug_compositor_create(struct weston_compositor
*compositor)
+{
+ struct weston_debug_compositor *wdc;
+
+ if (compositor->weston_debug)
+ return -1;
+
+ wdc = zalloc(sizeof *wdc);
+ if (!wdc)
+ return -1;
+
+ wdc->compositor = compositor;
+ wl_list_init(&wdc->scope_list);
+
+ compositor->weston_debug = wdc;
+
+ return 0;
+}
+
+/** Destroy weston_debug_compositor structure
+ *
+ * \param compositor The libweston compositor whose weston-debug to tear down.
+ *
+ * Clears weston_compositor::weston_debug.
+ *
+ * \internal
+ */
+void
+weston_debug_compositor_destroy(struct weston_compositor
*compositor)
+{
+ struct weston_debug_compositor *wdc = compositor-
Post by Daniel Stone
weston_debug;
+ struct weston_debug_scope *scope;
+
+ if (wdc->global)
+ wl_global_destroy(wdc->global);
+
+ wl_list_for_each(scope, &wdc->scope_list, compositor_link)
+ weston_log("Internal warning: debug scope '%s' has not
been destroyed.\n",
+ scope->name);
+
+ /* Remove head to not crash if scope removed later. */
+ wl_list_remove(&wdc->scope_list);
+
+ free(wdc);
+
+ compositor->weston_debug = NULL;
+}
+
+/** Enable weston-debug protocol extension
+ *
+ * \param compositor The libweston compositor where to enable.
+ *
+ * This enables the weston_debug_v1 Wayland protocol extension which any
+ * client can use to get debug messsages from the compositor.
+ *
+ * WARNING: This feature should not be used in production. If a client
+ * provides a file descriptor that blocks writes, it will block the whole
+ * compositor indefinitely.
+ *
+ * There is no control on which client is allowed to subscribe to debug
+ * messages. Any and all clients are allowed.
+ *
+ * The debug extension is disabled by default, and once enabled, cannot be
+ * disabled again.
+ */
+WL_EXPORT void
+weston_compositor_enable_debug_protocol(struct weston_compositor *compositor)
+{
+ struct weston_debug_compositor *wdc;
+
+ wdc = compositor->weston_debug;
+ if (!wdc)
+ return;
+
+ if (wdc->global)
+ return;
+
+ wdc->global = wl_global_create(compositor->wl_display,
+ &weston_debug_v1_interface, 1,
+ wdc, bind_weston_debug);
+ if (!wdc->global)
+ return;
+
+ weston_log("WARNING: debug protocol has been enabled. "
+ "This is a potential denial-of-service attack vector and "
+ "information leak.\n");
+}
+
+/** Register a new debug stream name, creating a debug scope
+ *
+ * \param compositor The libweston compositor where to add.
+ * \param name The debug stream/scope name.
+ * \param desc The debug scope description for humans.
+ * \param begin_cb Optional callback when a client subscribes to this scope.
+ * \param user_data Optional user data pointer for the callback.
+ * \return A valid pointer on success, NULL on failure.
+ *
+ * This function is used to create a debug scope. All debug message printing
+ * happens for a scope, which allows clients to subscribe to the kind of
+ * debug messages they want by \c name.
+ *
+ * \c name must be unique in the \c weston_compositor instance. \c name and
+ * \c description must both be provided. The description is printed when a
+ * client asks for a list of supported debug scopes.
+ *
+ * \c begin_cb, if not NULL, is called when a client subscribes to the
+ * debug scope creating a debug stream. This is for debug scopes that need
+ * to print messages as a response to a client appearing, e.g. printing a
+ * list of windows on demand or a static preamble. The argument \c user_data
+ * is passed in to the callback and is otherwise unused.
+ *
+ * For one-shot debug streams, \c begin_cb should finally call
+ * weston_debug_stream_complete() to close the stream and tell the client
+ * the printing is complete. Otherwise the client expects more to be written
+ * to its file descriptor.
+ *
+ * The debug scope must be destroyed before destroying the
+ * \c weston_compositor.
+ *
+ * \memberof weston_debug_scope
+ * \sa weston_debug_stream, weston_debug_scope_cb
+ */
+WL_EXPORT struct weston_debug_scope *
+weston_compositor_add_debug_scope(struct weston_compositor
*compositor,
+ const char *name,
+ const char *description,
+ weston_debug_scope_cb begin_cb,
+ void *user_data)
+{
+ struct weston_debug_compositor *wdc;
+ struct weston_debug_scope *scope;
+
+ if (!compositor || !name || !description) {
+ weston_log("Error: cannot add a debug scope without name
or description.\n");
+ return NULL;
+ }
+
+ wdc = compositor->weston_debug;
+ if (!wdc) {
+ weston_log("Error: cannot add debug scope '%s', infra not
initialized.\n",
+ name);
+ return NULL;
+ }
+
+ if (get_scope(wdc, name)){
+ weston_log("Error: debug scope named '%s' is already
registered.\n",
+ name);
+ return NULL;
+ }
+
+ scope = zalloc(sizeof *scope);
+ if (!scope) {
+ weston_log("Error adding debug scope '%s': out of
memory.\n",
+ name);
+ return NULL;
+ }
+
+ scope->name = strdup(name);
+ scope->desc = strdup(description);
+ scope->begin_cb = begin_cb;
+ scope->user_data = user_data;
+ wl_list_init(&scope->stream_list);
+
+ if (!scope->name || !scope->desc) {
+ weston_log("Error adding debug scope '%s': out of
memory.\n",
+ name);
+ free(scope->name);
+ free(scope->desc);
+ free(scope);
+ return NULL;
+ }
+
+ wl_list_insert(wdc->scope_list.prev, &scope->compositor_link);
+
+ return scope;
+}
+
+/** Destroy a debug scope
+ *
+ * \param scope The debug scope to destroy.
+ *
+ * Destroys the debug scope, closing all open streams subscribed to it and
+ * sending them each a \c weston_debug_stream_v1.failure event.
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT void
+weston_debug_scope_destroy(struct weston_debug_scope *scope)
+{
+ struct weston_debug_stream *stream;
+
+ if (!scope)
+ return;
+
+ while (!wl_list_empty(&scope->stream_list)) {
+ stream = wl_container_of(scope->stream_list.prev,
+ stream, scope_link);
+
+ stream_close_on_failure(stream, "debug name removed");
+ }
+
+ wl_list_remove(&scope->compositor_link);
+ free(scope->name);
+ free(scope->desc);
+ free(scope);
+}
+
+/** Are there any active subscriptions to the scope?
+ *
+ * \param scope The debug scope to check.
+ * \return True if any streams are open for this scope, false otherwise.
+ *
+ * As printing some debugging messages may be relatively expensive, one
+ * can use this function to determine if there is a need to gather the
+ * debugging information at all. If this function returns false, all
+ * printing for this scope is dropped, so gathering the information is
+ * pointless.
+ *
+ * The return value of this function should not be stored, as new clients
+ * may subscribe to the debug scope later.
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT bool
+weston_debug_scope_is_enabled(struct weston_debug_scope *scope)
+{
+ if (!scope)
+ return false;
+
+ return !wl_list_empty(&scope->stream_list);
+}
+
+/** Write data into a specific debug stream
+ *
+ * \param stream The debug stream to write into.
+ * \param data[in] Pointer to the data to write.
+ * \param len Number of bytes to write.
+ *
+ * Writes the given data (binary verbatim) into the debug stream.
+ * If \c len is zero or negative, the write is silently dropped.
+ *
+ * Writing is continued until all data has been written or
+ * a write fails. If the write fails due to a signal, it is re-tried.
+ * Otherwise on failure, the stream is closed and
+ * \c weston_debug_stream_v1.failure event is sent to the client.
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_write(struct weston_debug_stream *stream,
+ const char *data, size_t len)
+{
+ ssize_t len_ = len;
+ ssize_t ret;
+ int e;
+
+ if (stream->fd == -1)
+ return;
+
+ while (len_ > 0) {
+ ret = write(stream->fd, data, len_);
+ e = errno;
+ if (ret < 0) {
+ if (e == EINTR)
+ continue;
+
+ stream_close_on_failure(stream,
+ "Error writing %zd bytes: %s (%d)",
+ len_, strerror(e), e);
+ break;
+ }
+
+ len_ -= ret;
+ data += ret;
+ }
+}
+
+/** Write a formatted string into a specific debug stream (varargs)
+ *
+ * \param stream The debug stream to write into.
+ * \param fmt Printf-style format string.
+ * \param ap Formatting arguments.
+ *
+ * The behavioral details are the same as for
weston_debug_stream_write().
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_vprintf(struct weston_debug_stream *stream,
+ const char *fmt, va_list ap)
+{
+ char *str;
+ int len;
+
+ len = vasprintf(&str, fmt, ap);
+ if (len >= 0) {
+ weston_debug_stream_write(stream, str, len);
+ free(str);
+ } else {
+ stream_close_on_failure(stream, "Out of memory");
+ }
+}
+
+/** Write a formatted string into a specific debug stream
+ *
+ * \param stream The debug stream to write into.
+ * \param fmt Printf-style format string and arguments.
+ *
+ * The behavioral details are the same as for
weston_debug_stream_write().
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_printf(struct weston_debug_stream *stream,
+ const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ weston_debug_stream_vprintf(stream, fmt, ap);
+ va_end(ap);
+}
+
+/** Close the debug stream and send success event
+ *
+ * \param stream The debug stream to close.
+ *
+ * Closes the debug stream and sends \c
weston_debug_stream_v1.complete
+ * event to the client. This tells the client the debug information dump
+ * is complete.
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_complete(struct weston_debug_stream *stream)
+{
+ stream_close_unlink(stream);
+ weston_debug_stream_v1_send_complete(stream->resource);
+}
+
+/** Write debug data for a scope
+ *
+ * \param scope The debug scope to write for.
+ * \param data[in] Pointer to the data to write.
+ * \param len Number of bytes to write.
+ *
+ * Writes the given data to all subscribed clients' streams.
+ *
+ * The behavioral details for each stream are the same as for
+ * weston_debug_stream_write().
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT void
+weston_debug_scope_write(struct weston_debug_scope *scope,
+ const char *data, size_t len)
+{
+ struct weston_debug_stream *stream;
+
+ if (!scope)
+ return;
+
+ wl_list_for_each(stream, &scope->stream_list, scope_link)
+ weston_debug_stream_write(stream, data, len);
+}
+
+/** Write a formatted string for a scope (varargs)
+ *
+ * \param scope The debug scope to write for.
+ * \param fmt Printf-style format string.
+ * \param ap Formatting arguments.
+ *
+ * Writes to formatted string to all subscribed clients' streams.
+ *
+ * The behavioral details for each stream are the same as for
+ * weston_debug_stream_write().
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT void
+weston_debug_scope_vprintf(struct weston_debug_scope *scope,
+ const char *fmt, va_list ap)
+{
+ static const char oom[] = "Out of memory";
+ char *str;
+ int len;
+
+ if (!weston_debug_scope_is_enabled(scope))
+ return;
+
+ len = vasprintf(&str, fmt, ap);
+ if (len >= 0) {
+ weston_debug_scope_write(scope, str, len);
+ free(str);
+ } else {
+ weston_debug_scope_write(scope, oom, sizeof oom - 1);
+ }
+}
+
+/** Write a formatted string for a scope
+ *
+ * \param scope The debug scope to write for.
+ * \param fmt Printf-style format string and arguments.
+ *
+ * Writes to formatted string to all subscribed clients' streams.
+ *
+ * The behavioral details for each stream are the same as for
+ * weston_debug_stream_write().
+ *
+ * \memberof weston_debug_scope
+ */
+WL_EXPORT void
+weston_debug_scope_printf(struct weston_debug_scope *scope,
+ const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ weston_debug_scope_vprintf(scope, fmt, ap);
+ va_end(ap);
+}
+
+/** Format current time as a string
+ * and append the debug scope name to it
+ *
+ * \param scope[in] debug scope.
+ * \param buf[out] Buffer to store the string.
+ * \param len Available size in the buffer in bytes.
+ * \return \c buf
+ *
+ * Reads the current local wall-clock time and formats it into a string.
+ * and append the debug scope name to it.
+ * The string is nul-terminated, even if truncated.
+ */
+WL_EXPORT char *
+weston_debug_scope_timestamp(struct weston_debug_scope *scope,
+ char *buf, size_t len)
+{
+ struct timeval tv;
+ struct tm *bdt;
+ char string[128];
+ size_t ret = 0;
+
+ gettimeofday(&tv, NULL);
+
+ bdt = localtime(&tv.tv_sec);
+ if (bdt)
+ ret = strftime(string, sizeof string,
+ "%Y-%m-%d %H:%M:%S", bdt);
+
+ if (ret > 0)
+ snprintf(buf, len, "[%s.%03ld][%s]", string,
+ tv.tv_usec / 1000, scope->name);
+ else
+ snprintf(buf, len, "[?][%s]", scope->name);
+
+ return buf;
+}
diff --git a/libweston/weston-debug.h b/libweston/weston-debug.h
new file mode 100644
index 000000000..c76cec852
--- /dev/null
+++ b/libweston/weston-debug.h
@@ -0,0 +1,107 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef WESTON_DEBUG_H
+#define WESTON_DEBUG_H
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdarg.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct weston_compositor;
+
+void
+weston_compositor_enable_debug_protocol(struct weston_compositor *);
+
+struct weston_debug_scope;
+struct weston_debug_stream;
+
+/** weston_debug_scope callback
+ *
+ * \param stream The debug stream.
+ * \param user_data The \c user_data argument given to
+ * weston_compositor_add_debug_scope()
+ *
+ * \memberof weston_debug_scope
+ * \sa weston_debug_stream
+ */
+typedef void (*weston_debug_scope_cb)(struct weston_debug_stream *stream,
+ void *user_data);
+
+struct weston_debug_scope *
+weston_compositor_add_debug_scope(struct weston_compositor
*compositor,
+ const char *name,
+ const char *description,
+ weston_debug_scope_cb begin_cb,
+ void *user_data);
+
+void
+weston_debug_scope_destroy(struct weston_debug_scope *scope);
+
+bool
+weston_debug_scope_is_enabled(struct weston_debug_scope *scope);
+
+void
+weston_debug_scope_write(struct weston_debug_scope *scope,
+ const char *data, size_t len);
+
+void
+weston_debug_scope_vprintf(struct weston_debug_scope *scope,
+ const char *fmt, va_list ap);
+
+void
+weston_debug_scope_printf(struct weston_debug_scope *scope,
+ const char *fmt, ...)
+ __attribute__ ((format (printf, 2, 3)));
+
+void
+weston_debug_stream_write(struct weston_debug_stream *stream,
+ const char *data, size_t len);
+
+void
+weston_debug_stream_vprintf(struct weston_debug_stream *stream,
+ const char *fmt, va_list ap);
+
+void
+weston_debug_stream_printf(struct weston_debug_stream *stream,
+ const char *fmt, ...)
+ __attribute__ ((format (printf, 2, 3)));
+
+void
+weston_debug_stream_complete(struct weston_debug_stream *stream);
+
+char *
+weston_debug_scope_timestamp(struct weston_debug_scope *scope,
+ char *buf, size_t len);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* WESTON_DEBUG_H */
--
2.17.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-08-03 14:04:41 UTC
Permalink
On Fri, 20 Jul 2018 20:03:24 +0100
Post by Daniel Stone
weston_debug is both a libweston API for relaying debugging messages,
and the compositor-debug wayland protocol implementation for accessing those
debug messages from a Wayland client.
weston_debug_compositor_{create,destroy}() are private API, hence not
exported.
append the debug scope name along with the timestamp in
weston_debug_scope_timestamp API
Add explicit advertisement of debug scope names.
---
Makefile.am | 2 +
libweston/compositor.c | 6 +
libweston/compositor.h | 9 +
libweston/weston-debug.c | 723 +++++++++++++++++++++++++++++++++++++++
libweston/weston-debug.h | 107 ++++++
5 files changed, 847 insertions(+)
create mode 100644 libweston/weston-debug.c
create mode 100644 libweston/weston-debug.h
...
Post by Daniel Stone
diff --git a/libweston/weston-debug.c b/libweston/weston-debug.c
new file mode 100644
index 000000000..039247f14
--- /dev/null
+++ b/libweston/weston-debug.c
@@ -0,0 +1,723 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include "weston-debug.h"
+#include "helpers.h"
+#include "compositor.h"
+
+#include "weston-debug-server-protocol.h"
+
+#include <unistd.h>
+#include <stdarg.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/time.h>
+
+/** Main weston-debug context
+ *
+ * One per weston_compositor.
+ *
+ * \internal
+ */
+struct weston_debug_compositor {
+ struct weston_compositor *compositor;
+ struct wl_listener compositor_destroy_listener;
+ struct wl_global *global;
+ struct wl_list scope_list; /**< weston_debug_scope::compositor_link */
+
+ struct weston_debug_scope *list;
Hi Daniel,

'list' member is now unused, replaced by the explicit advertisement
events.
Post by Daniel Stone
+};
+
+/** weston-debug message scope
+ *
+ * This is used for scoping debugging messages. Clients can subscribe to
+ * only the scopes they are interested in. A scope is identified by its name
+ * (also referred to as debug stream name).
+ */
+struct weston_debug_scope {
+ char *name;
+ char *desc;
'desc' is never used anymore. This was meant to give the human looking
at the list of debug scopes a description of what they are, but looks
like the protocol does not carry it.

Do we not need it?
Post by Daniel Stone
+ weston_debug_scope_cb begin_cb;
+ void *user_data;
+ struct wl_list stream_list; /**< weston_debug_stream::scope_link */
+ struct wl_list compositor_link;
+};
+
+/** A debug stream created by a client
+ *
+ * A client provides a file descriptor for the server to write debug
+ * messages into. A weston_debug_stream is associated to one
+ * weston_debug_scope via the scope name, and the scope provides the messages.
+ * There can be several streams for the same scope, all streams getting the
+ * same messages.
+ */
+struct weston_debug_stream {
+ int fd; /**< client provided fd */
+ struct wl_resource *resource; /**< weston_debug_stream_v1 object */
+ struct wl_list scope_link;
+};
...
Post by Daniel Stone
+static void
+bind_weston_debug(struct wl_client *client,
+ void *data, uint32_t version, uint32_t id)
+{
+ struct weston_debug_compositor *wdc = data;
+ struct weston_debug_scope *scope;
+ struct wl_resource *resource;
+
+ resource = wl_resource_create(client,
+ &weston_debug_v1_interface,
+ version, id);
+ if (!resource) {
+ wl_client_post_no_memory(client);
+ return;
+ }
+ wl_resource_set_implementation(resource, &weston_debug_impl,
+ wdc, NULL);
+
+ wl_list_for_each(scope, &wdc->scope_list, compositor_link)
+ weston_debug_v1_send_available(resource, scope->name);
Whitespace issues in indentation.


Otherwise looks good to me.


Thanks,
pq
Pekka Paalanen
2018-08-06 11:15:38 UTC
Permalink
On Fri, 20 Jul 2018 20:03:24 +0100
Post by Daniel Stone
weston_debug is both a libweston API for relaying debugging messages,
and the compositor-debug wayland protocol implementation for accessing those
debug messages from a Wayland client.
weston_debug_compositor_{create,destroy}() are private API, hence not
exported.
append the debug scope name along with the timestamp in
weston_debug_scope_timestamp API
Add explicit advertisement of debug scope names.
---
Makefile.am | 2 +
libweston/compositor.c | 6 +
libweston/compositor.h | 9 +
libweston/weston-debug.c | 723 +++++++++++++++++++++++++++++++++++++++
libweston/weston-debug.h | 107 ++++++
5 files changed, 847 insertions(+)
create mode 100644 libweston/weston-debug.c
create mode 100644 libweston/weston-debug.h
...
Post by Daniel Stone
+/** Format current time as a string
+ * and append the debug scope name to it
+ *
+ * \param scope[in] debug scope.
+ * \param buf[out] Buffer to store the string.
+ * \param len Available size in the buffer in bytes.
+ * \return \c buf
+ *
+ * Reads the current local wall-clock time and formats it into a string.
+ * and append the debug scope name to it.
+ * The string is nul-terminated, even if truncated.
+ */
+WL_EXPORT char *
+weston_debug_scope_timestamp(struct weston_debug_scope *scope,
+ char *buf, size_t len)
+{
+ struct timeval tv;
+ struct tm *bdt;
+ char string[128];
+ size_t ret = 0;
+
+ gettimeofday(&tv, NULL);
+
+ bdt = localtime(&tv.tv_sec);
+ if (bdt)
+ ret = strftime(string, sizeof string,
+ "%Y-%m-%d %H:%M:%S", bdt);
+
+ if (ret > 0)
+ snprintf(buf, len, "[%s.%03ld][%s]", string,
+ tv.tv_usec / 1000, scope->name);
+ else
+ snprintf(buf, len, "[?][%s]", scope->name);
+
+ return buf;
+}
Hi,

realized something when looking at the "log" debug scope patch:
weston_debug_scope_timestamp() should probably be resilient against
scope == NULL, all the other functions already are.

weston_log gets initialized early, but the log debug scope gets
initialized after the compositor. If something logs something between
those two...


Thanks,
pq
Daniel Stone
2018-08-23 16:53:07 UTC
Permalink
Hi Pekka,
Post by Pekka Paalanen
Post by Daniel Stone
+/** Format current time as a string
+ * and append the debug scope name to it
+ *
+ * \param scope[in] debug scope.
+ * \param buf[out] Buffer to store the string.
+ * \param len Available size in the buffer in bytes.
+ * \return \c buf
+ *
+ * Reads the current local wall-clock time and formats it into a string.
+ * and append the debug scope name to it.
+ * The string is nul-terminated, even if truncated.
+ */
+WL_EXPORT char *
+weston_debug_scope_timestamp(struct weston_debug_scope *scope,
+ char *buf, size_t len)
+{
+ struct timeval tv;
+ struct tm *bdt;
+ char string[128];
+ size_t ret = 0;
+
+ gettimeofday(&tv, NULL);
+
+ bdt = localtime(&tv.tv_sec);
+ if (bdt)
+ ret = strftime(string, sizeof string,
+ "%Y-%m-%d %H:%M:%S", bdt);
+
+ if (ret > 0)
+ snprintf(buf, len, "[%s.%03ld][%s]", string,
+ tv.tv_usec / 1000, scope->name);
+ else
+ snprintf(buf, len, "[?][%s]", scope->name);
+
+ return buf;
+}
weston_debug_scope_timestamp() should probably be resilient against
scope == NULL, all the other functions already are.
weston_log gets initialized early, but the log debug scope gets
initialized after the compositor. If something logs something between
those two...
The users introduced in this patchset already check if the scope is
enabled, which bails out if the scope is NULL. Given that, and that I
can't see a sensible behaviour if the scope is NULL, I'm inclined to
add an assert(scope) at the top of the function as well as
documentation. Does that seem reasonable? Would assert(scope) be
better or assert(weston_debug_scope_is_enabled(scope))?

Cheers,
Daniel
Pekka Paalanen
2018-08-24 08:12:39 UTC
Permalink
On Thu, 23 Aug 2018 17:53:07 +0100
Post by Daniel Stone
Hi Pekka,
Post by Pekka Paalanen
Post by Daniel Stone
+/** Format current time as a string
+ * and append the debug scope name to it
+ *
+ * \param scope[in] debug scope.
+ * \param buf[out] Buffer to store the string.
+ * \param len Available size in the buffer in bytes.
+ * \return \c buf
+ *
+ * Reads the current local wall-clock time and formats it into a string.
+ * and append the debug scope name to it.
+ * The string is nul-terminated, even if truncated.
+ */
+WL_EXPORT char *
+weston_debug_scope_timestamp(struct weston_debug_scope *scope,
+ char *buf, size_t len)
+{
+ struct timeval tv;
+ struct tm *bdt;
+ char string[128];
+ size_t ret = 0;
+
+ gettimeofday(&tv, NULL);
+
+ bdt = localtime(&tv.tv_sec);
+ if (bdt)
+ ret = strftime(string, sizeof string,
+ "%Y-%m-%d %H:%M:%S", bdt);
+
+ if (ret > 0)
+ snprintf(buf, len, "[%s.%03ld][%s]", string,
+ tv.tv_usec / 1000, scope->name);
+ else
+ snprintf(buf, len, "[?][%s]", scope->name);
+
+ return buf;
+}
weston_debug_scope_timestamp() should probably be resilient against
scope == NULL, all the other functions already are.
weston_log gets initialized early, but the log debug scope gets
initialized after the compositor. If something logs something between
those two...
The users introduced in this patchset already check if the scope is
enabled, which bails out if the scope is NULL. Given that, and that I
can't see a sensible behaviour if the scope is NULL, I'm inclined to
add an assert(scope) at the top of the function as well as
documentation. Does that seem reasonable? Would assert(scope) be
better or assert(weston_debug_scope_is_enabled(scope))?
Hi Daniel,

after "compositor: offer logs via weston-debug",
main.c:custom_handler() will call weston_debug_scope_timestamp()
unchecked.

Relying on libwayland not logging anything until the debug scope has
been set up seems a bit too fragile to me.

We could forbid calling weston_debug_scope_timestamp() with NULL scope,
but I think it would be easier if it just accepted NULL by doing
whatever non-crashy. That would be easier for developers. It could
simply replace the scope name with "unknown" for instance. The
resulting string will probably never be used, so it might just make an
empty string too.



On another matter, I've been wondering if storing the debug scopes in
weston_compositor is a good thing. We have the logger initialized
earlier, and quite a lot might happen before weston_compositor is
created. Also the idea of a circular buffer for after-the-fact
reporting might benefit from having debug scopes set up early, and the
command line argument to enable debug scopes from launch.


Thanks,
pq
Daniel Stone
2018-08-27 15:20:41 UTC
Permalink
Hi Pekka,
Post by Pekka Paalanen
Post by Daniel Stone
The users introduced in this patchset already check if the scope is
enabled, which bails out if the scope is NULL. Given that, and that I
can't see a sensible behaviour if the scope is NULL, I'm inclined to
add an assert(scope) at the top of the function as well as
documentation. Does that seem reasonable? Would assert(scope) be
better or assert(weston_debug_scope_is_enabled(scope))?
after "compositor: offer logs via weston-debug",
main.c:custom_handler() will call weston_debug_scope_timestamp()
unchecked.
Relying on libwayland not logging anything until the debug scope has
been set up seems a bit too fragile to me.
We could forbid calling weston_debug_scope_timestamp() with NULL scope,
but I think it would be easier if it just accepted NULL by doing
whatever non-crashy. That would be easier for developers. It could
simply replace the scope name with "unknown" for instance. The
resulting string will probably never be used, so it might just make an
empty string too.
OK, good point. Generally I hate doing nonsensical/non-helpful things
in unexpected conditions we shouldn't be called, but I don't see an
easy way out of this one for now.
Post by Pekka Paalanen
On another matter, I've been wondering if storing the debug scopes in
weston_compositor is a good thing. We have the logger initialized
earlier, and quite a lot might happen before weston_compositor is
created. Also the idea of a circular buffer for after-the-fact
reporting might benefit from having debug scopes set up early, and the
command line argument to enable debug scopes from launch.
Yeah, I think we can definitely rework this to push the scope of the
debug stuff up. As noted in
https://gitlab.freedesktop.org/wayland/weston/issues/133 I think it
would be really good to have all this information as early as
possible, and maybe even outliving the compositor. From a libweston
API point of view, having the same API for general logging as we do
for debug makes a huge amount of sense.

Given how invasive that would be, my suggestion would be that we track
that as a follow-up issue. Once we've got all the functionality in
place we can see for sure what the best API/structure would be.

Cheers,
Daniel
Pekka Paalanen
2018-08-28 14:33:38 UTC
Permalink
On Mon, 27 Aug 2018 16:20:41 +0100
Post by Daniel Stone
Hi Pekka,
Post by Pekka Paalanen
On another matter, I've been wondering if storing the debug scopes in
weston_compositor is a good thing. We have the logger initialized
earlier, and quite a lot might happen before weston_compositor is
created. Also the idea of a circular buffer for after-the-fact
reporting might benefit from having debug scopes set up early, and the
command line argument to enable debug scopes from launch.
Yeah, I think we can definitely rework this to push the scope of the
debug stuff up. As noted in
https://gitlab.freedesktop.org/wayland/weston/issues/133 I think it
would be really good to have all this information as early as
possible, and maybe even outliving the compositor. From a libweston
API point of view, having the same API for general logging as we do
for debug makes a huge amount of sense.
Given how invasive that would be, my suggestion would be that we track
that as a follow-up issue. Once we've got all the functionality in
place we can see for sure what the best API/structure would be.
Agreed.


Thanks,
pq

Pekka Paalanen
2018-08-06 13:05:42 UTC
Permalink
On Fri, 20 Jul 2018 20:03:24 +0100
Post by Daniel Stone
weston_debug is both a libweston API for relaying debugging messages,
and the compositor-debug wayland protocol implementation for accessing those
debug messages from a Wayland client.
weston_debug_compositor_{create,destroy}() are private API, hence not
exported.
append the debug scope name along with the timestamp in
weston_debug_scope_timestamp API
Add explicit advertisement of debug scope names.
---
Makefile.am | 2 +
libweston/compositor.c | 6 +
libweston/compositor.h | 9 +
libweston/weston-debug.c | 723 +++++++++++++++++++++++++++++++++++++++
libweston/weston-debug.h | 107 ++++++
5 files changed, 847 insertions(+)
create mode 100644 libweston/weston-debug.c
create mode 100644 libweston/weston-debug.h
Hi,

yet another thing I noticed.
Post by Daniel Stone
+/** Write data into a specific debug stream
+ *
+ * \param stream The debug stream to write into.
+ * \param data[in] Pointer to the data to write.
+ * \param len Number of bytes to write.
+ *
+ * Writes the given data (binary verbatim) into the debug stream.
+ * If \c len is zero or negative, the write is silently dropped.
+ *
+ * Writing is continued until all data has been written or
+ * a write fails. If the write fails due to a signal, it is re-tried.
+ * Otherwise on failure, the stream is closed and
+ * \c weston_debug_stream_v1.failure event is sent to the client.
+ *
+ * \memberof weston_debug_stream
+ */
+WL_EXPORT void
+weston_debug_stream_write(struct weston_debug_stream *stream,
+ const char *data, size_t len)
+{
+ ssize_t len_ = len;
+ ssize_t ret;
+ int e;
+
+ if (stream->fd == -1)
+ return;
+
+ while (len_ > 0) {
+ ret = write(stream->fd, data, len_);
+ e = errno;
+ if (ret < 0) {
+ if (e == EINTR)
+ continue;
+
+ stream_close_on_failure(stream,
+ "Error writing %zd bytes: %s (%d)",
+ len_, strerror(e), e);
+ break;
+ }
+
+ len_ -= ret;
+ data += ret;
If write() starts returning zero, we might be in for a very long loop.
Post by Daniel Stone
+ }
+}
Thanks,
pq
Daniel Stone
2018-08-27 15:15:23 UTC
Permalink
Hi Pekka,
Post by Pekka Paalanen
+ while (len_ > 0) {
+ ret = write(stream->fd, data, len_);
+ e = errno;
+ if (ret < 0) {
+ if (e == EINTR)
+ continue;
+
+ stream_close_on_failure(stream,
+ "Error writing %zd bytes: %s (%d)",
+ len_, strerror(e), e);
+ break;
+ }
+
+ len_ -= ret;
+ data += ret;
If write() starts returning zero, we might be in for a very long loop.
I don't think there's much we can do about that to be honest. I guess
we could post an error to the owning client and disconnect it? Else
we're just throwing away data, potentially corrupting the debug
stream, which seems suboptimal.

Currently we just block, and document that your debug client _must_
keep up with the incoming stream.

Cheers,
Daniel
Pekka Paalanen
2018-08-28 14:30:14 UTC
Permalink
On Mon, 27 Aug 2018 16:15:23 +0100
Post by Daniel Stone
Hi Pekka,
Post by Pekka Paalanen
+ while (len_ > 0) {
+ ret = write(stream->fd, data, len_);
+ e = errno;
+ if (ret < 0) {
+ if (e == EINTR)
+ continue;
+
+ stream_close_on_failure(stream,
+ "Error writing %zd bytes: %s (%d)",
+ len_, strerror(e), e);
+ break;
+ }
+
+ len_ -= ret;
+ data += ret;
If write() starts returning zero, we might be in for a very long loop.
I don't think there's much we can do about that to be honest. I guess
we could post an error to the owning client and disconnect it? Else
we're just throwing away data, potentially corrupting the debug
stream, which seems suboptimal.
Currently we just block, and document that your debug client _must_
keep up with the incoming stream.
Hi Daniel,

yes, so if write() returns zero, something is really wrong, since the
blocking behaviour is supposed to prevent that, right?

I think disconnecting the client is better than spinning forever or
losing bits of data, but is there a case where we should spin at least
a little? Implementing a timeout seems overkill and bad for other
reasons, so if occasional zero returns are normal, then the code is ok
as is.

There is also 'failure' event we could use if disconnecting is too much.

I'm also ok with not caring about this issue until someone hits it.


Thanks,
pq
Daniel Stone
2018-07-20 19:03:23 UTC
Permalink
From: Pekka Paalanen <***@iki.fi>

This is a new debugging extension for non-production environments. The
aim is to replace all build-time choosable debug prints in the
compositor with runtime subscribable debug streams.

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

Added new libweston-$MAJOR-protocols.pc file and install that
for external projects to find the XML files installed by libweston.

Signed-off-by: Maniraj Devadoss <***@in.bosch.com>

Use noarch_pkgconfig_DATA instead, add ${pc_sysrootdir}, drop
unnecessary EXTRA_DIST of weston-debug.xml.

Signed-off-by: Pekka Paalanen <***@collabora.co.uk>

Add explicit advertisement of available debug interfaces.

Signed-off-by: Daniel Stone <***@collabora.com>
---
Makefile.am | 10 ++
configure.ac | 1 +
libweston/libweston-protocols.pc.in | 7 ++
protocol/weston-debug.xml | 136 ++++++++++++++++++++++++++++
4 files changed, 154 insertions(+)
create mode 100644 libweston/libweston-protocols.pc.in
create mode 100644 protocol/weston-debug.xml

diff --git a/Makefile.am b/Makefile.am
index 3bce47a11..66eb365c5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -106,6 +106,10 @@ ***@LIBWESTON_MAJOR@_la_SOURCES = \
shared/platform.h \
shared/weston-egl-ext.h

+***@LIBWESTON_MAJOR@_datadir = $(datadir)/weston/protocols
+***@LIBWESTON_MAJOR@_data_DATA = \
+ protocol/weston-debug.xml
+
lib_LTLIBRARIES += libweston-desktop-@***@.la
***@LIBWESTON_MAJOR@_la_CPPFLAGS = $(AM_CPPFLAGS) -DIN_WESTON
***@LIBWESTON_MAJOR@_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
@@ -152,6 +156,8 @@ endif
***@LIBWESTON_MAJOR@_la_SOURCES = \
protocol/weston-screenshooter-protocol.c \
protocol/weston-screenshooter-server-protocol.h \
+ protocol/weston-debug-protocol.c \
+ protocol/weston-debug-server-protocol.h \
protocol/text-cursor-position-protocol.c \
protocol/text-cursor-position-server-protocol.h \
protocol/text-input-unstable-v1-protocol.c \
@@ -276,6 +282,10 @@ pkgconfig_DATA = \
libweston-desktop/libweston-desktop-${LIBWESTON_MAJOR}.pc \
compositor/weston.pc

+noarch_pkgconfigdir = $(datadir)/pkgconfig
+noarch_pkgconfig_DATA = \
+ libweston/libweston-${LIBWESTON_MAJOR}-protocols.pc
+
wayland_sessiondir = $(datadir)/wayland-sessions
dist_wayland_session_DATA = compositor/weston.desktop

diff --git a/configure.ac b/configure.ac
index 093d6b549..3a138ea91 100644
--- a/configure.ac
+++ b/configure.ac
@@ -695,6 +695,7 @@ AC_CONFIG_FILES([Makefile libweston/version.h compositor/weston.pc])
# libweston_abi_version here, and outside [] because of m4 quoting rules
AC_CONFIG_FILES([libweston/libweston-]libweston_major_version[.pc:libweston/libweston.pc.in])
AC_CONFIG_FILES([libweston/libweston-]libweston_major_version[-uninstalled.pc:libweston/libweston-uninstalled.pc.in])
+AC_CONFIG_FILES([libweston/libweston-]libweston_major_version[-protocols.pc:libweston/libweston-protocols.pc.in])
AC_CONFIG_FILES([libweston-desktop/libweston-desktop-]libweston_major_version[.pc:libweston-desktop/libweston-desktop.pc.in])
AC_CONFIG_FILES([libweston-desktop/libweston-desktop-]libweston_major_version[-uninstalled.pc:libweston-desktop/libweston-desktop-uninstalled.pc.in])

diff --git a/libweston/libweston-protocols.pc.in b/libweston/libweston-protocols.pc.in
new file mode 100644
index 000000000..6547a0d5a
--- /dev/null
+++ b/libweston/libweston-protocols.pc.in
@@ -0,0 +1,7 @@
+prefix=@prefix@
+datarootdir=@datarootdir@
+pkgdatadir=${pc_sysrootdir}@datadir@/@PACKAGE@/protocols
+
+Name: libWeston Protocols
+Description: libWeston protocol files
+Version: @WESTON_VERSION@
diff --git a/protocol/weston-debug.xml b/protocol/weston-debug.xml
new file mode 100644
index 000000000..1154c3ae2
--- /dev/null
+++ b/protocol/weston-debug.xml
@@ -0,0 +1,136 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="weston_debug">
+
+ <copyright>
+ Copyright © 2017 Pekka Paalanen ***@iki.fi
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the "Software"),
+ to deal in the Software without restriction, including without limitation
+ the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ and/or sell copies of the Software, and to permit persons to whom the
+ Software is furnished to do so, subject to the following conditions:
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="weston_debug_v1" version="1">
+ <description summary="weston internal debugging">
+ This is a generic debugging interface for Weston internals, the global
+ object advertized through wl_registry.
+
+ WARNING: This interface by design allows a denial-of-service attack. It
+ should not be offered in production, or proper authorization mechnisms
+ must be enforced.
+
+ The idea is for a client to provide a file descriptor that the server
+ uses for printing debug information. The server uses the file
+ descriptor in blocking writes mode, which exposes the denial-of-service
+ risk. The blocking mode is necessary to ensure all debug messages can
+ be easily printed in place. It also ensures message ordering if a
+ client subcribes to more than one debug stream.
+
+ The available debugging features depend on the server.
+
+ A debug stream can be one-shot where the server prints the requested
+ information and then closes it, or continuous where server keeps on
+ printing until the client stops it. Or anything in between.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy factory object">
+ Destroys the factory object, but does not affect any other objects.
+ </description>
+ </request>
+
+ <event name="available">
+ <description summary="advertise available debug scope">
+ Advertises an available debug scope which the client may be able to
+ bind to. No information is provided by the server about the content
+ contained within the debug streams provided by the scope, once a
+ client has subscribed.
+ </description>
+
+ <arg name="name" type="string" allow-null="false"
+ summary="debug stream name"/>
+ </event>
+
+ <request name="subscribe">
+ <description summary="subscribe to a debug stream">
+ Subscribe to a named debug stream. The server will start printing
+ to the given file descriptor.
+
+ If the named debug stream is a one-shot dump, the server will send
+ weston_debug_stream_v1.complete event once all requested data has
+ been printed. Otherwise, the server will continue streaming debug
+ prints until the subscription object is destroyed.
+
+ If the debug stream name is unknown to the server, the server will
+ immediately respond with weston_debug_stream_v1.failure event.
+ </description>
+
+ <arg name="name" type="string" allow-null="false"
+ summary="debug stream name"/>
+ <arg name="streamfd" type="fd" summary="write stream file descriptor"/>
+ <arg name="stream" type="new_id" interface="weston_debug_stream_v1"
+ summary="created debug stream object"/>
+ </request>
+ </interface>
+
+ <interface name="weston_debug_stream_v1" version="1">
+ <description summary="A subscribed debug stream">
+ Represents one subscribed debug stream, created with
+ weston_debug_v1.subscribe. When the object is created, it is associated
+ with a given file descriptor. The server will continue writing to the
+ file descriptor until the object is destroyed or the server sends an
+ event through the object.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="close a debug stream">
+ Destroys the object, which causes the server to stop writing into
+ and closes the associated file descriptor if it was not closed
+ already.
+
+ Use a wl_display.sync if the clients needs to guarantee the file
+ descriptor is closed before continuing.
+ </description>
+ </request>
+
+ <event name="complete">
+ <description summary="server completed the debug stream">
+ The server has successfully finished writing to and has closed the
+ associated file descriptor.
+
+ This event is delivered only for one-shot debug streams where the
+ server dumps some data and stop. This is never delivered for
+ continuous debbug streams because they by definition never complete.
+ </description>
+ </event>
+
+ <event name="failure">
+ <description summary="server cannot continue the debug stream">
+ The server has stopped writing to and has closed the
+ associated file descriptor. The data already written to the file
+ descriptor is correct, but it may be truncated.
+
+ This event may be delivered at any time and for any kind of debug
+ stream. It may be due to a failure in or shutdown of the server.
+ The message argument may provide a hint of the reason.
+ </description>
+
+ <arg name="message" type="string" allow-null="true"
+ summary="human readable reason"/>
+ </event>
+ </interface>
+</protocol>
--
2.17.1
Ucan, Emre (ADITG/ESB)
2018-07-26 14:22:12 UTC
Permalink
Reviewed-by: Emre Ucan <***@de.adit-jv.com>

Best regards

Emre Ucan
Engineering Software Base (ADITG/ESB)

Tel. +49 5121 49 6937
-----Original Message-----
From: wayland-devel [mailto:wayland-devel-
Sent: Freitag, 20. Juli 2018 21:06
Subject: [PATCH weston v5 02/14] protocol: add weston-debug.xml
This is a new debugging extension for non-production environments. The
aim is to replace all build-time choosable debug prints in the
compositor with runtime subscribable debug streams.
Added new libweston-$MAJOR-protocols.pc file and install that
for external projects to find the XML files installed by libweston.
Use noarch_pkgconfig_DATA instead, add ${pc_sysrootdir}, drop
unnecessary EXTRA_DIST of weston-debug.xml.
Add explicit advertisement of available debug interfaces.
---
Makefile.am | 10 ++
configure.ac | 1 +
libweston/libweston-protocols.pc.in | 7 ++
protocol/weston-debug.xml | 136 ++++++++++++++++++++++++++++
4 files changed, 154 insertions(+)
create mode 100644 libweston/libweston-protocols.pc.in
create mode 100644 protocol/weston-debug.xml
diff --git a/Makefile.am b/Makefile.am
index 3bce47a11..66eb365c5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -106,6 +106,10 @@ ***@LIBWESTON_MAJOR@_la_SOURCES =
\
shared/platform.h \
shared/weston-egl-ext.h
+ protocol/weston-debug.xml
+
$(AM_CPPFLAGS) -DIN_WESTON
$(COMPOSITOR_CFLAGS)
@@ -152,6 +156,8 @@ endif
\
protocol/weston-screenshooter-protocol.c \
protocol/weston-screenshooter-server-protocol.h
\
+ protocol/weston-debug-protocol.c \
+ protocol/weston-debug-server-protocol.h
\
protocol/text-cursor-position-protocol.c \
protocol/text-cursor-position-server-protocol.h \
protocol/text-input-unstable-v1-protocol.c \
@@ -276,6 +282,10 @@ pkgconfig_DATA = \
libweston-desktop/libweston-desktop-${LIBWESTON_MAJOR}.pc \
compositor/weston.pc
+noarch_pkgconfigdir = $(datadir)/pkgconfig
+noarch_pkgconfig_DATA = \
+ libweston/libweston-${LIBWESTON_MAJOR}-protocols.pc
+
wayland_sessiondir = $(datadir)/wayland-sessions
dist_wayland_session_DATA = compositor/weston.desktop
diff --git a/configure.ac b/configure.ac
index 093d6b549..3a138ea91 100644
--- a/configure.ac
+++ b/configure.ac
@@ -695,6 +695,7 @@ AC_CONFIG_FILES([Makefile libweston/version.h compositor/weston.pc])
# libweston_abi_version here, and outside [] because of m4 quoting rules
AC_CONFIG_FILES([libweston/libweston-
]libweston_major_version[.pc:libweston/libweston.pc.in])
AC_CONFIG_FILES([libweston/libweston-]libweston_major_version[-
uninstalled.pc:libweston/libweston-uninstalled.pc.in])
+AC_CONFIG_FILES([libweston/libweston-]libweston_major_version[-
protocols.pc:libweston/libweston-protocols.pc.in])
AC_CONFIG_FILES([libweston-desktop/libweston-desktop-
]libweston_major_version[.pc:libweston-desktop/libweston-desktop.pc.in])
AC_CONFIG_FILES([libweston-desktop/libweston-desktop-
]libweston_major_version[-uninstalled.pc:libweston-desktop/libweston-
desktop-uninstalled.pc.in])
diff --git a/libweston/libweston-protocols.pc.in b/libweston/libweston-
protocols.pc.in
new file mode 100644
index 000000000..6547a0d5a
--- /dev/null
+++ b/libweston/libweston-protocols.pc.in
@@ -0,0 +1,7 @@
+
+Name: libWeston Protocols
+Description: libWeston protocol files
diff --git a/protocol/weston-debug.xml b/protocol/weston-debug.xml
new file mode 100644
index 000000000..1154c3ae2
--- /dev/null
+++ b/protocol/weston-debug.xml
@@ -0,0 +1,136 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="weston_debug">
+
+ <copyright>
+
+ Permission is hereby granted, free of charge, to any person obtaining a
+ copy of this software and associated documentation files (the "Software"),
+ to deal in the Software without restriction, including without limitation
+ the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ and/or sell copies of the Software, and to permit persons to whom the
+
+ The above copyright notice and this permission notice (including the next
+ paragraph) shall be included in all copies or substantial portions of the
+ Software.
+
+ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
MERCHANTABILITY,
+ FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
DAMAGES OR OTHER
+ LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ DEALINGS IN THE SOFTWARE.
+ </copyright>
+
+ <interface name="weston_debug_v1" version="1">
+ <description summary="weston internal debugging">
+ This is a generic debugging interface for Weston internals, the global
+ object advertized through wl_registry.
+
+ WARNING: This interface by design allows a denial-of-service attack. It
+ should not be offered in production, or proper authorization mechnisms
+ must be enforced.
+
+ The idea is for a client to provide a file descriptor that the server
+ uses for printing debug information. The server uses the file
+ descriptor in blocking writes mode, which exposes the denial-of-service
+ risk. The blocking mode is necessary to ensure all debug messages can
+ be easily printed in place. It also ensures message ordering if a
+ client subcribes to more than one debug stream.
+
+ The available debugging features depend on the server.
+
+ A debug stream can be one-shot where the server prints the requested
+ information and then closes it, or continuous where server keeps on
+ printing until the client stops it. Or anything in between.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="destroy factory object">
+ Destroys the factory object, but does not affect any other objects.
+ </description>
+ </request>
+
+ <event name="available">
+ <description summary="advertise available debug scope">
+ Advertises an available debug scope which the client may be able to
+ bind to. No information is provided by the server about the content
+ contained within the debug streams provided by the scope, once a
+ client has subscribed.
+ </description>
+
+ <arg name="name" type="string" allow-null="false"
+ summary="debug stream name"/>
+ </event>
+
+ <request name="subscribe">
+ <description summary="subscribe to a debug stream">
+ Subscribe to a named debug stream. The server will start printing
+ to the given file descriptor.
+
+ If the named debug stream is a one-shot dump, the server will send
+ weston_debug_stream_v1.complete event once all requested data has
+ been printed. Otherwise, the server will continue streaming debug
+ prints until the subscription object is destroyed.
+
+ If the debug stream name is unknown to the server, the server will
+ immediately respond with weston_debug_stream_v1.failure event.
+ </description>
+
+ <arg name="name" type="string" allow-null="false"
+ summary="debug stream name"/>
+ <arg name="streamfd" type="fd" summary="write stream file descriptor"/>
+ <arg name="stream" type="new_id"
interface="weston_debug_stream_v1"
+ summary="created debug stream object"/>
+ </request>
+ </interface>
+
+ <interface name="weston_debug_stream_v1" version="1">
+ <description summary="A subscribed debug stream">
+ Represents one subscribed debug stream, created with
+ weston_debug_v1.subscribe. When the object is created, it is associated
+ with a given file descriptor. The server will continue writing to the
+ file descriptor until the object is destroyed or the server sends an
+ event through the object.
+ </description>
+
+ <request name="destroy" type="destructor">
+ <description summary="close a debug stream">
+ Destroys the object, which causes the server to stop writing into
+ and closes the associated file descriptor if it was not closed
+ already.
+
+ Use a wl_display.sync if the clients needs to guarantee the file
+ descriptor is closed before continuing.
+ </description>
+ </request>
+
+ <event name="complete">
+ <description summary="server completed the debug stream">
+ The server has successfully finished writing to and has closed the
+ associated file descriptor.
+
+ This event is delivered only for one-shot debug streams where the
+ server dumps some data and stop. This is never delivered for
+ continuous debbug streams because they by definition never
complete.
+ </description>
+ </event>
+
+ <event name="failure">
+ <description summary="server cannot continue the debug stream">
+ The server has stopped writing to and has closed the
+ associated file descriptor. The data already written to the file
+ descriptor is correct, but it may be truncated.
+
+ This event may be delivered at any time and for any kind of debug
+ stream. It may be due to a failure in or shutdown of the server.
+ The message argument may provide a hint of the reason.
+ </description>
+
+ <arg name="message" type="string" allow-null="true"
+ summary="human readable reason"/>
+ </event>
+ </interface>
+</protocol>
--
2.17.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Daniel Stone
2018-07-20 19:03:29 UTC
Permalink
From: Pekka Paalanen <***@iki.fi>

This is preparation for using the weston-debug infrastructure for
WM_DEBUG. dump_property() may be called from different debugging
contexts and often needs to be prefixed with more information.

An alternative to this patch would be to pass in the weston_debug_scope
as an argument to dump_property(), but then all callers would need to be
converted to weston-debug infra in a single commit.

Therefore require the callers to provide the FILE* to print to.

Signed-off-by: Pekka Paalanen <***@iki.fi>
Signed-off-by: Maniraj Devadoss <***@in.bosch.com>
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>
Reviewed-by: Daniel Stone <***@collabora.com>
---
xwayland/selection.c | 39 +++++++++++++++++++++--
xwayland/window-manager.c | 67 ++++++++++++++++-----------------------
xwayland/xwayland.h | 3 +-
3 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/xwayland/selection.c b/xwayland/selection.c
index a75557044..85c320c29 100644
--- a/xwayland/selection.c
+++ b/xwayland/selection.c
@@ -34,6 +34,12 @@
#include "xwayland.h"
#include "shared/helpers.h"

+#ifdef WM_DEBUG
+#define wm_log(...) weston_log(__VA_ARGS__)
+#else
+#define wm_log(...) do {} while (0)
+#endif
+
static int
writable_callback(int fd, uint32_t mask, void *data)
{
@@ -102,6 +108,9 @@ weston_wm_get_incr_chunk(struct weston_wm *wm)
{
xcb_get_property_cookie_t cookie;
xcb_get_property_reply_t *reply;
+ FILE *fp;
+ char *logstr;
+ size_t logsize;

cookie = xcb_get_property(wm->conn,
0, /* delete */
@@ -115,7 +124,13 @@ weston_wm_get_incr_chunk(struct weston_wm *wm)
if (reply == NULL)
return;

- dump_property(wm, wm->atom.wl_selection, reply);
+ fp = open_memstream(&logstr, &logsize);
+ if (fp) {
+ dump_property(fp, wm, wm->atom.wl_selection, reply);
+ if (fclose(fp) == 0)
+ wm_log("%s", logstr);
+ free(logstr);
+ }

if (xcb_get_property_value_length(reply) > 0) {
/* reply's ownership is transferred to wm, which is responsible
@@ -178,6 +193,9 @@ weston_wm_get_selection_targets(struct weston_wm *wm)
xcb_atom_t *value;
char **p;
uint32_t i;
+ FILE *fp;
+ char *logstr;
+ size_t logsize;

cookie = xcb_get_property(wm->conn,
1, /* delete */
@@ -191,7 +209,13 @@ weston_wm_get_selection_targets(struct weston_wm *wm)
if (reply == NULL)
return;

- dump_property(wm, wm->atom.wl_selection, reply);
+ fp = open_memstream(&logstr, &logsize);
+ if (fp) {
+ dump_property(fp, wm, wm->atom.wl_selection, reply);
+ if (fclose(fp) == 0)
+ wm_log("%s", logstr);
+ free(logstr);
+ }

if (reply->type != XCB_ATOM_ATOM) {
free(reply);
@@ -232,6 +256,9 @@ weston_wm_get_selection_data(struct weston_wm *wm)
{
xcb_get_property_cookie_t cookie;
xcb_get_property_reply_t *reply;
+ FILE *fp;
+ char *logstr;
+ size_t logsize;

cookie = xcb_get_property(wm->conn,
1, /* delete */
@@ -243,7 +270,13 @@ weston_wm_get_selection_data(struct weston_wm *wm)

reply = xcb_get_property_reply(wm->conn, cookie, NULL);

- dump_property(wm, wm->atom.wl_selection, reply);
+ fp = open_memstream(&logstr, &logsize);
+ if (fp) {
+ dump_property(fp, wm, wm->atom.wl_selection, reply);
+ if (fclose(fp) == 0)
+ wm_log("%s", logstr);
+ free(logstr);
+ }

if (reply == NULL) {
return;
diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 3bf323a42..4a26f6e7f 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -210,23 +210,6 @@ wm_log(const char *fmt, ...)
#endif
}

-static int __attribute__ ((format (printf, 1, 2)))
-wm_log_continue(const char *fmt, ...)
-{
-#ifdef WM_DEBUG
- int l;
- va_list argp;
-
- va_start(argp, fmt);
- l = weston_vlog_continue(fmt, argp);
- va_end(argp);
-
- return l;
-#else
- return 0;
-#endif
-}
-
static void
weston_output_weak_ref_init(struct weston_output_weak_ref *ref)
{
@@ -430,7 +413,7 @@ dump_cardinal_array(FILE *fp, xcb_get_property_reply_t *reply)
}

void
-dump_property(struct weston_wm *wm,
+dump_property(FILE *fp, struct weston_wm *wm,
xcb_atom_t property, xcb_get_property_reply_t *reply)
{
int32_t *incr_value;
@@ -439,18 +422,11 @@ dump_property(struct weston_wm *wm,
xcb_window_t *window_value;
int width, len;
uint32_t i;
- FILE *fp;
- char *logstr;
- size_t logsize;
-
- fp = open_memstream(&logstr, &logsize);
- if (!fp)
- return;

width = fprintf(fp, "%s: ", get_atom_name(wm->conn, property));
if (reply == NULL) {
fprintf(fp, "(no reply)\n");
- goto out;
+ return;
}

width += fprintf(fp, "%s/%d, length %d (value_len %d): ",
@@ -492,15 +468,10 @@ dump_property(struct weston_wm *wm,
} else {
fprintf(fp, "huh?\n");
}
-
-out:
- if (fclose(fp) == 0)
- wm_log_continue("%s", logstr);
- free(logstr);
}

static void
-read_and_dump_property(struct weston_wm *wm,
+read_and_dump_property(FILE *fp, struct weston_wm *wm,
xcb_window_t window, xcb_atom_t property)
{
xcb_get_property_reply_t *reply;
@@ -510,7 +481,7 @@ read_and_dump_property(struct weston_wm *wm,
property, XCB_ATOM_ANY, 0, 2048);
reply = xcb_get_property_reply(wm->conn, cookie, NULL);

- dump_property(wm, property, reply);
+ dump_property(fp, wm, property, reply);

free(reply);
}
@@ -1389,19 +1360,35 @@ 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;
+ char *logstr;
+ size_t logsize;

if (!wm_lookup_window(wm, property_notify->window, &window))
return;

window->properties_dirty = 1;

- wm_log("XCB_PROPERTY_NOTIFY: window %d, ", property_notify->window);
- if (property_notify->state == XCB_PROPERTY_DELETE)
- wm_log_continue("deleted %s\n",
- get_atom_name(wm->conn, property_notify->atom));
- else
- read_and_dump_property(wm, property_notify->window,
- property_notify->atom);
+ fp = open_memstream(&logstr, &logsize);
+ if (fp) {
+ fprintf(fp, "XCB_PROPERTY_NOTIFY: window %d, ", property_notify->window);
+ if (property_notify->state == XCB_PROPERTY_DELETE)
+ fprintf(fp, "deleted %s\n",
+ get_atom_name(wm->conn, property_notify->atom));
+ else
+ read_and_dump_property(fp, wm, property_notify->window,
+ property_notify->atom);
+
+ if (fclose(fp) == 0)
+ wm_log("%s", logstr);
+ free(logstr);
+ } else {
+ /* read_and_dump_property() is a X11 roundtrip.
+ * Mimic it to maintain ordering semantics between debug
+ * and non-debug paths.
+ */
+ get_atom_name(wm->conn, property_notify->atom);
+ }

if (property_notify->atom == wm->atom.net_wm_name ||
property_notify->atom == XCB_ATOM_WM_NAME)
diff --git a/xwayland/xwayland.h b/xwayland/xwayland.h
index ca75f5b7c..52da67869 100644
--- a/xwayland/xwayland.h
+++ b/xwayland/xwayland.h
@@ -23,6 +23,7 @@
* SOFTWARE.
*/

+#include <stdio.h>
#include <wayland-server.h>
#include <xcb/xcb.h>
#include <xcb/xfixes.h>
@@ -159,7 +160,7 @@ struct weston_wm {
};

void
-dump_property(struct weston_wm *wm, xcb_atom_t property,
+dump_property(FILE *fp, struct weston_wm *wm, xcb_atom_t property,
xcb_get_property_reply_t *reply);

const char *
--
2.17.1
Daniel Stone
2018-07-20 19:03:30 UTC
Permalink
From: Pekka Paalanen <***@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 <***@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 <***@in.bosch.com>
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>
Reviewed-by: Daniel Stone <***@collabora.com>
---
xwayland/launcher.c | 7 ++
xwayland/window-manager.c | 166 ++++++++++++++++++++------------------
xwayland/xwayland.h | 3 +
3 files changed, 99 insertions(+), 77 deletions(-)

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 {
--
2.17.1
Daniel Stone
2018-07-20 19:03:33 UTC
Permalink
Add a 'scene-graph' debug scope which will dump out the current set of
outputs, layers, and views and as much information as possible about how
they are rendered and composited.

Signed-off-by: Daniel Stone <***@collabora.com>
---
libweston/compositor.c | 219 +++++++++++++++++++++++++++++++++++++++++
libweston/compositor.h | 4 +
2 files changed, 223 insertions(+)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 0c147d4a6..dea91d173 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -55,6 +55,8 @@
#include "timeline.h"

#include "compositor.h"
+#include "weston-debug.h"
+#include "linux-dmabuf.h"
#include "viewporter-server-protocol.h"
#include "presentation-time-server-protocol.h"
#include "shared/helpers.h"
@@ -6306,6 +6308,216 @@ timeline_key_binding_handler(struct weston_keyboard *keyboard,
weston_timeline_open(compositor);
}

+static const char *
+output_repaint_status_text(struct weston_output *output)
+{
+ switch (output->repaint_status) {
+ case REPAINT_NOT_SCHEDULED:
+ return "no repaint";
+ case REPAINT_BEGIN_FROM_IDLE:
+ return "start_repaint_loop scheduled";
+ case REPAINT_SCHEDULED:
+ return "repaint scheduled";
+ case REPAINT_AWAITING_COMPLETION:
+ return "awaiting completion";
+ }
+
+ assert(!"output_repaint_status_text missing enum");
+ return NULL;
+}
+
+static void
+debug_scene_view_print_buffer(FILE *fp, struct weston_view *view)
+{
+ struct weston_buffer *buffer = view->surface->buffer_ref.buffer;
+ struct wl_shm_buffer *shm;
+ struct linux_dmabuf_buffer *dmabuf;
+
+ if (!buffer) {
+ fprintf(fp, "\t\tsolid-colour surface\n");
+ return;
+ }
+
+ shm = wl_shm_buffer_get(buffer->resource);
+ if (shm) {
+ fprintf(fp, "\t\tSHM buffer\n");
+ fprintf(fp, "\t\t\tformat: 0x%lx\n",
+ (unsigned long) wl_shm_buffer_get_format(shm));
+ return;
+ }
+
+ dmabuf = linux_dmabuf_buffer_get(buffer->resource);
+ if (dmabuf) {
+ fprintf(fp, "\t\tdmabuf buffer\n");
+ fprintf(fp, "\t\t\tformat: 0x%lx\n",
+ (unsigned long) dmabuf->attributes.format);
+ fprintf(fp, "\t\t\tmodifier: 0x%llx\n",
+ (unsigned long long) dmabuf->attributes.modifier[0]);
+ return;
+ }
+
+ fprintf(fp, "\t\tEGL buffer");
+}
+
+static void
+debug_scene_view_print(FILE *fp, struct weston_view *view, int view_idx)
+{
+ struct weston_compositor *ec = view->surface->compositor;
+ struct weston_output *output;
+ pixman_box32_t *box;
+ uint32_t surface_id = 0;
+ pid_t pid = 0;
+
+ if (view->surface->resource) {
+ struct wl_resource *resource = view->surface->resource;
+ wl_client_get_credentials(wl_resource_get_client(resource),
+ &pid, NULL, NULL);
+ surface_id = wl_resource_get_id(view->surface->resource);
+ }
+
+ fprintf(fp, "\tView %d (role %s, PID %d, surface ID %u, %p):\n",
+ view_idx, view->surface->role_name, pid, surface_id, view);
+
+ box = pixman_region32_extents(&view->transform.boundingbox);
+ fprintf(fp, "\t\tposition: (%d, %d) -> (%d, %d)\n",
+ box->x1, box->y1, box->x2, box->y2);
+ box = pixman_region32_extents(&view->transform.opaque);
+
+ if (pixman_region32_equal(&view->transform.opaque,
+ &view->transform.boundingbox)) {
+ fprintf(fp, "\t\t[fully opaque]\n");
+ } else if (!pixman_region32_not_empty(&view->transform.opaque)) {
+ fprintf(fp, "\t\t[not opaque]\n");
+ } else {
+ fprintf(fp, "\t\t[opaque: (%d, %d) -> (%d, %d)]\n",
+ box->x1, box->y1, box->x2, box->y2);
+ }
+
+ if (view->alpha < 1.0)
+ fprintf(fp, "\t\talpha: %f\n", view->alpha);
+
+ if (view->output_mask != 0) {
+ fprintf(fp, "\t\toutputs: ");
+ wl_list_for_each(output, &ec->output_list, link) {
+ if (!(view->output_mask & (1 << output->id)))
+ continue;
+ fprintf(fp, "%s%d (%s)%s",
+ (view->output_mask & ((1 << output->id) - 1)) ? ", " : "",
+ output->id, output->name,
+ (view->output == output) ? " (primary)" : "");
+ }
+ } else {
+ fprintf(fp, "\t\t[no outputs]");
+ }
+
+ fprintf(fp, "\n");
+
+ debug_scene_view_print_buffer(fp, view);
+}
+
+/**
+ * Output information on how libweston is currently composing the scene
+ * graph.
+ */
+WL_EXPORT char *
+weston_compositor_print_scene_graph(struct weston_compositor *ec)
+{
+ struct weston_output *output;
+ struct weston_layer *layer;
+ struct timespec now;
+ int layer_idx = 0;
+ FILE *fp;
+ char *ret;
+ size_t len;
+ int err;
+
+ fp = open_memstream(&ret, &len);
+ assert(fp);
+
+ weston_compositor_read_presentation_clock(ec, &now);
+ fprintf(fp, "Weston scene graph at %ld.%09ld:\n\n",
+ now.tv_sec, now.tv_nsec);
+
+ wl_list_for_each(output, &ec->output_list, link) {
+ struct weston_head *head;
+ int head_idx = 0;
+
+ fprintf(fp, "Output %d (%s):\n", output->id, output->name);
+ if (!output->enabled) {
+ fprintf(fp, "disabled\n");
+ continue;
+ }
+
+ fprintf(fp, "\tposition: (%d, %d) -> (%d, %d)\n",
+ output->x, output->y,
+ output->x + output->width,
+ output->y + output->height);
+ fprintf(fp, "\tmode: %dx%d@%fHz\n",
+ output->current_mode->width,
+ output->current_mode->height,
+ output->current_mode->refresh / 1000.0);
+ fprintf(fp, "\tscale: %d\n", output->scale);
+
+ fprintf(fp, "\trepaint status: %s\n",
+ output_repaint_status_text(output));
+ if (output->repaint_status == REPAINT_SCHEDULED)
+ fprintf(fp, "\tnext repaint: %ld.%09ld\n",
+ output->next_repaint.tv_sec,
+ output->next_repaint.tv_nsec);
+
+ wl_list_for_each(head, &output->head_list, output_link) {
+ fprintf(fp, "\tHead %d (%s): %sconnected\n",
+ head_idx++, head->name,
+ (head->connected) ? "" : "not ");
+ }
+ }
+
+ fprintf(fp, "\n");
+
+ wl_list_for_each(layer, &ec->layer_list, link) {
+ struct weston_view *view;
+ int view_idx = 0;
+
+ fprintf(fp, "Layer %d (pos 0x%lx):\n", layer_idx++,
+ (unsigned long) layer->position);
+
+ if (!weston_layer_mask_is_infinite(layer)) {
+ fprintf(fp, "\t[mask: (%d, %d) -> (%d,%d)]\n\n",
+ layer->mask.x1, layer->mask.y1,
+ layer->mask.x2, layer->mask.y2);
+ }
+
+ wl_list_for_each(view, &layer->view_list.link, layer_link.link)
+ debug_scene_view_print(fp, view, view_idx++);
+
+ if (wl_list_empty(&layer->view_list.link))
+ fprintf(fp, "\t[no views]\n");
+
+ fprintf(fp, "\n");
+ }
+
+ err = fclose(fp);
+ assert(err == 0);
+
+ return ret;
+}
+
+/**
+ * Called when the 'scene-graph' debug scope is bound by a client. This
+ * one-shot weston-debug scope prints the current scene graph when bound,
+ * and then terminates the stream.
+ */
+static void
+debug_scene_graph_cb(struct weston_debug_stream *stream, void *data)
+{
+ struct weston_compositor *ec = data;
+ char *str = weston_compositor_print_scene_graph(ec);
+
+ weston_debug_stream_printf(stream, "%s", str);
+ free(str);
+ weston_debug_stream_complete(stream);
+}
+
/** Create the compositor.
*
* This functions creates and initializes a compositor instance.
@@ -6415,6 +6627,12 @@ weston_compositor_create(struct wl_display *display, void *user_data)
weston_compositor_add_debug_binding(ec, KEY_T,
timeline_key_binding_handler, ec);

+ ec->debug_scene =
+ weston_compositor_add_debug_scope(ec, "scene-graph",
+ "Scene graph details\n",
+ debug_scene_graph_cb,
+ ec);
+
return ec;

fail:
@@ -6715,6 +6933,7 @@ weston_compositor_destroy(struct weston_compositor *compositor)
if (compositor->heads_changed_source)
wl_event_source_remove(compositor->heads_changed_source);

+ weston_debug_scope_destroy(compositor->debug_scene);
weston_debug_compositor_destroy(compositor);

free(compositor);
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 67a835713..669ce821e 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1164,6 +1164,7 @@ struct weston_compositor {
struct weston_touch_calibrator *touch_calibrator;

struct weston_debug_compositor *weston_debug;
+ struct weston_debug_scope *debug_scene;
};

struct weston_buffer {
@@ -1934,6 +1935,9 @@ weston_buffer_reference(struct weston_buffer_reference *ref,
void
weston_compositor_get_time(struct timespec *time);

+char *
+weston_compositor_print_scene_graph(struct weston_compositor *ec);
+
void
weston_compositor_destroy(struct weston_compositor *ec);
struct weston_compositor *
--
2.17.1
Pekka Paalanen
2018-08-06 15:11:30 UTC
Permalink
On Fri, 20 Jul 2018 20:03:33 +0100
Post by Daniel Stone
Add a 'scene-graph' debug scope which will dump out the current set of
outputs, layers, and views and as much information as possible about how
they are rendered and composited.
---
libweston/compositor.c | 219 +++++++++++++++++++++++++++++++++++++++++
libweston/compositor.h | 4 +
2 files changed, 223 insertions(+)
Hi Daniel,

this looks nice, just a few minor issues below.
Post by Daniel Stone
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 0c147d4a6..dea91d173 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -55,6 +55,8 @@
#include "timeline.h"
#include "compositor.h"
+#include "weston-debug.h"
+#include "linux-dmabuf.h"
#include "viewporter-server-protocol.h"
#include "presentation-time-server-protocol.h"
#include "shared/helpers.h"
@@ -6306,6 +6308,216 @@ timeline_key_binding_handler(struct weston_keyboard *keyboard,
weston_timeline_open(compositor);
}
+static const char *
+output_repaint_status_text(struct weston_output *output)
+{
+ switch (output->repaint_status) {
+ return "no repaint";
+ return "start_repaint_loop scheduled";
+ return "repaint scheduled";
+ return "awaiting completion";
+ }
+
+ assert(!"output_repaint_status_text missing enum");
+ return NULL;
+}
+
+static void
+debug_scene_view_print_buffer(FILE *fp, struct weston_view *view)
+{
+ struct weston_buffer *buffer = view->surface->buffer_ref.buffer;
+ struct wl_shm_buffer *shm;
+ struct linux_dmabuf_buffer *dmabuf;
+
+ if (!buffer) {
+ fprintf(fp, "\t\tsolid-colour surface\n");
This is incorrect. Under GL-renderer, it will claim all wl_shm surfaces
to be solid color surfaces, e.g. background, panel, and a
weston-terminal window.
Post by Daniel Stone
+ return;
+ }
+
+ shm = wl_shm_buffer_get(buffer->resource);
+ if (shm) {
+ fprintf(fp, "\t\tSHM buffer\n");
+ fprintf(fp, "\t\t\tformat: 0x%lx\n",
+ (unsigned long) wl_shm_buffer_get_format(shm));
+ return;
+ }
+
+ dmabuf = linux_dmabuf_buffer_get(buffer->resource);
+ if (dmabuf) {
+ fprintf(fp, "\t\tdmabuf buffer\n");
+ fprintf(fp, "\t\t\tformat: 0x%lx\n",
+ (unsigned long) dmabuf->attributes.format);
+ fprintf(fp, "\t\t\tmodifier: 0x%llx\n",
+ (unsigned long long) dmabuf->attributes.modifier[0]);
+ return;
+ }
+
+ fprintf(fp, "\t\tEGL buffer");
+}
+
+static void
+debug_scene_view_print(FILE *fp, struct weston_view *view, int view_idx)
+{
+ struct weston_compositor *ec = view->surface->compositor;
+ struct weston_output *output;
+ pixman_box32_t *box;
+ uint32_t surface_id = 0;
+ pid_t pid = 0;
+
+ if (view->surface->resource) {
+ struct wl_resource *resource = view->surface->resource;
+ wl_client_get_credentials(wl_resource_get_client(resource),
+ &pid, NULL, NULL);
+ surface_id = wl_resource_get_id(view->surface->resource);
+ }
+
+ fprintf(fp, "\tView %d (role %s, PID %d, surface ID %u, %p):\n",
+ view_idx, view->surface->role_name, pid, surface_id, view);
It might be useful here to print the surface description using
weston_surface::get_label() vfunc. For an example, see
check_weston_surface_description() in timeline.c.

Printing role_name is nice, it pointed out that desktop-shell is not
calling weston_surface_set_role() backgrounds or panels. I don't see
why it shouldn't.
Post by Daniel Stone
+
+ box = pixman_region32_extents(&view->transform.boundingbox);
+ fprintf(fp, "\t\tposition: (%d, %d) -> (%d, %d)\n",
+ box->x1, box->y1, box->x2, box->y2);
+ box = pixman_region32_extents(&view->transform.opaque);
+
+ if (pixman_region32_equal(&view->transform.opaque,
+ &view->transform.boundingbox)) {
+ fprintf(fp, "\t\t[fully opaque]\n");
+ } else if (!pixman_region32_not_empty(&view->transform.opaque)) {
+ fprintf(fp, "\t\t[not opaque]\n");
This case could also mean transformed with a matrix, but I suppose we're
interested in how the renderer is handling the surface, not how a
client set it up?
Post by Daniel Stone
+ } else {
+ fprintf(fp, "\t\t[opaque: (%d, %d) -> (%d, %d)]\n",
+ box->x1, box->y1, box->x2, box->y2);
+ }
+
+ if (view->alpha < 1.0)
+ fprintf(fp, "\t\talpha: %f\n", view->alpha);
+
+ if (view->output_mask != 0) {
+ fprintf(fp, "\t\toutputs: ");
+ wl_list_for_each(output, &ec->output_list, link) {
+ if (!(view->output_mask & (1 << output->id)))
+ continue;
+ fprintf(fp, "%s%d (%s)%s",
+ (view->output_mask & ((1 << output->id) - 1)) ? ", " : "",
I don't think output_list is guaranteed to be ordered by output id, the
bit allocator chooses the lowest free bit IIRC. But that's cosmetic.
Post by Daniel Stone
+ output->id, output->name,
+ (view->output == output) ? " (primary)" : "");
+ }
+ } else {
+ fprintf(fp, "\t\t[no outputs]");
+ }
+
+ fprintf(fp, "\n");
+
+ debug_scene_view_print_buffer(fp, view);
+}
+
+/**
+ * Output information on how libweston is currently composing the scene
+ * graph.
+ */
+WL_EXPORT char *
+weston_compositor_print_scene_graph(struct weston_compositor *ec)
+{
+ struct weston_output *output;
+ struct weston_layer *layer;
+ struct timespec now;
+ int layer_idx = 0;
+ FILE *fp;
+ char *ret;
+ size_t len;
+ int err;
+
+ fp = open_memstream(&ret, &len);
+ assert(fp);
+
+ weston_compositor_read_presentation_clock(ec, &now);
+ fprintf(fp, "Weston scene graph at %ld.%09ld:\n\n",
+ now.tv_sec, now.tv_nsec);
+
+ wl_list_for_each(output, &ec->output_list, link) {
+ struct weston_head *head;
+ int head_idx = 0;
+
+ fprintf(fp, "Output %d (%s):\n", output->id, output->name);
+ if (!output->enabled) {
+ fprintf(fp, "disabled\n");
+ continue;
I think disabled outputs should never be on output_list.
Post by Daniel Stone
+ }
+
+ fprintf(fp, "\tposition: (%d, %d) -> (%d, %d)\n",
+ output->x, output->y,
+ output->x + output->width,
+ output->y + output->height);
Three decimals would be enough.

The rest is good.
Post by Daniel Stone
+ output->current_mode->width,
+ output->current_mode->height,
+ output->current_mode->refresh / 1000.0);
+ fprintf(fp, "\tscale: %d\n", output->scale);
+
+ fprintf(fp, "\trepaint status: %s\n",
+ output_repaint_status_text(output));
+ if (output->repaint_status == REPAINT_SCHEDULED)
+ fprintf(fp, "\tnext repaint: %ld.%09ld\n",
+ output->next_repaint.tv_sec,
+ output->next_repaint.tv_nsec);
+
+ wl_list_for_each(head, &output->head_list, output_link) {
+ fprintf(fp, "\tHead %d (%s): %sconnected\n",
+ head_idx++, head->name,
+ (head->connected) ? "" : "not ");
+ }
+ }
...


Thanks,
pq
Daniel Stone
2018-07-20 19:03:26 UTC
Permalink
From: Pekka Paalanen <***@iki.fi>

A tool for accessing the zcompositor_debug_v1 interface features.

Installed along weston-info, because it should be potentially useful for
people running libweston-based compositors.

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

Added a man page for weston-debug client

Signed-off-by: Maniraj Devadoss <***@in.bosch.com>
[Pekka: fixed 'missing braces aroudn initializer' warning]

Add --list and --all arguments, using interface advertisement.

Signed-off-by: Daniel Stone <***@collabora.com>
---
Makefile.am | 16 +-
clients/weston-debug.c | 453 +++++++++++++++++++++++++++++++++++++++++
man/weston-debug.man | 46 +++++
3 files changed, 513 insertions(+), 2 deletions(-)
create mode 100644 clients/weston-debug.c
create mode 100644 man/weston-debug.man

diff --git a/Makefile.am b/Makefile.am
index c2d9048b3..04381e0f7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -531,7 +531,7 @@ spring_tool_SOURCES = \

if BUILD_CLIENTS

-bin_PROGRAMS += weston-terminal weston-info
+bin_PROGRAMS += weston-terminal weston-info weston-debug

libexec_PROGRAMS += \
weston-desktop-shell \
@@ -862,6 +862,15 @@ nodist_weston_info_SOURCES = \
weston_info_LDADD = $(WESTON_INFO_LIBS) libshared.la
weston_info_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)

+weston_debug_SOURCES = \
+ clients/weston-debug.c \
+ shared/helpers.h
+nodist_weston_debug_SOURCES = \
+ protocol/weston-debug-protocol.c \
+ protocol/weston-debug-client-protocol.h
+weston_debug_LDADD = $(WESTON_INFO_LIBS) libshared.la
+weston_debug_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
+
weston_desktop_shell_SOURCES = \
clients/desktop-shell.c \
shared/helpers.h
@@ -898,6 +907,8 @@ BUILT_SOURCES += \
protocol/weston-screenshooter-client-protocol.h \
protocol/weston-touch-calibration-protocol.c \
protocol/weston-touch-calibration-client-protocol.h \
+ protocol/weston-debug-protocol.c \
+ protocol/weston-debug-client-protocol.h \
protocol/text-cursor-position-client-protocol.h \
protocol/text-cursor-position-protocol.c \
protocol/text-input-unstable-v1-protocol.c \
@@ -1595,7 +1606,7 @@ surface_screenshot_la_SOURCES = tests/surface-screenshot.c
# Documentation
#

-man_MANS = weston.1 weston.ini.5
+man_MANS = weston.1 weston.ini.5 weston-debug.1

if ENABLE_DRM_COMPOSITOR
man_MANS += weston-drm.7
@@ -1623,6 +1634,7 @@ SUFFIXES = .1 .5 .7 .man
EXTRA_DIST += \
doc/calibration-helper.bash \
man/weston.man \
+ man/weston-debug.man \
man/weston-drm.man \
man/weston-rdp.man \
man/weston.ini.man
diff --git a/clients/weston-debug.c b/clients/weston-debug.c
new file mode 100644
index 000000000..59dacd269
--- /dev/null
+++ b/clients/weston-debug.c
@@ -0,0 +1,453 @@
+/*
+ * Copyright © 2017 Pekka Paalanen <***@iki.fi>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include <errno.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include <wayland-client.h>
+
+#include "shared/helpers.h"
+#include "shared/zalloc.h"
+#include "weston-debug-client-protocol.h"
+
+struct debug_app {
+ struct {
+ bool help;
+ bool list;
+ bool bind_all;
+ char *output;
+ char *outfd;
+ } opt;
+
+ int out_fd;
+ struct wl_display *dpy;
+ struct wl_registry *registry;
+ struct weston_debug_v1 *debug_iface;
+ struct wl_list stream_list;
+};
+
+struct debug_stream {
+ struct wl_list link;
+ bool should_bind;
+ char *name;
+ struct weston_debug_stream_v1 *obj;
+};
+
+static struct debug_stream *
+stream_alloc(struct debug_app *app, const char *name)
+{
+ struct debug_stream *stream;
+
+ stream = zalloc(sizeof *stream);
+ if (!stream)
+ return NULL;
+
+ stream->name = strdup(name);
+ if (!stream->name) {
+ free(stream);
+ return NULL;
+ }
+
+ stream->should_bind = app->opt.bind_all;
+ wl_list_insert(app->stream_list.prev, &stream->link);
+
+ return stream;
+}
+
+static struct debug_stream *
+stream_find(struct debug_app *app, const char *name)
+{
+ struct debug_stream *stream;
+
+ wl_list_for_each(stream, &app->stream_list, link) {
+ if (strcmp(stream->name, name) == 0)
+ return stream;
+ }
+
+ return stream_alloc(app, name);
+}
+
+static void
+stream_destroy(struct debug_stream *stream)
+{
+ if (stream->obj)
+ weston_debug_stream_v1_destroy(stream->obj);
+
+ wl_list_remove(&stream->link);
+ free(stream->name);
+ free(stream);
+}
+
+static void
+destroy_streams(struct debug_app *app)
+{
+ struct debug_stream *stream;
+ struct debug_stream *tmp;
+
+ wl_list_for_each_safe(stream, tmp, &app->stream_list, link)
+ stream_destroy(stream);
+}
+
+static void
+debug_advertise(void *data, struct weston_debug_v1 *debug, const char *name)
+{
+ struct debug_app *app = data;
+ (void) stream_find(app, name);
+}
+
+static const struct weston_debug_v1_listener debug_listener = {
+ debug_advertise,
+};
+
+static void
+global_handler(void *data, struct wl_registry *registry, uint32_t id,
+ const char *interface, uint32_t version)
+{
+ struct debug_app *app = data;
+ uint32_t myver;
+
+ assert(app->registry == registry);
+
+ if (!strcmp(interface, weston_debug_v1_interface.name)) {
+ if (app->debug_iface)
+ return;
+
+ myver = MIN(1, version);
+ app->debug_iface =
+ wl_registry_bind(registry, id,
+ &weston_debug_v1_interface, myver);
+ weston_debug_v1_add_listener(app->debug_iface, &debug_listener,
+ app);
+ }
+}
+
+static void
+global_remove_handler(void *data, struct wl_registry *registry, uint32_t name)
+{
+}
+
+static const struct wl_registry_listener registry_listener = {
+ global_handler,
+ global_remove_handler
+};
+
+static void
+handle_stream_complete(void *data, struct weston_debug_stream_v1 *obj)
+{
+ struct debug_stream *stream = data;
+
+ assert(stream->obj == obj);
+
+ stream_destroy(stream);
+}
+
+static void
+handle_stream_failure(void *data, struct weston_debug_stream_v1 *obj,
+ const char *msg)
+{
+ struct debug_stream *stream = data;
+
+ assert(stream->obj == obj);
+
+ fprintf(stderr, "Debug stream '%s' aborted: %s\n", stream->name, msg);
+
+ stream_destroy(stream);
+}
+
+static const struct weston_debug_stream_v1_listener stream_listener = {
+ handle_stream_complete,
+ handle_stream_failure
+};
+
+static void
+start_streams(struct debug_app *app)
+{
+ struct debug_stream *stream;
+
+ wl_list_for_each(stream, &app->stream_list, link) {
+ if (!stream->should_bind)
+ continue;
+
+ stream->obj = weston_debug_v1_subscribe(app->debug_iface,
+ stream->name,
+ app->out_fd);
+ weston_debug_stream_v1_add_listener(stream->obj,
+ &stream_listener, stream);
+ }
+}
+
+/* Returns true if execution should continue, false to exit */
+static bool
+list_streams(struct debug_app *app)
+{
+ struct debug_stream *stream;
+ bool ret = false;
+
+ fprintf(stderr, "Available debug streams:\n");
+
+ wl_list_for_each(stream, &app->stream_list, link) {
+ if (stream->should_bind)
+ ret = true;
+ fprintf(stderr, " %s [will %sbind]\n", stream->name,
+ (stream->should_bind) ? "" : "not ");
+ }
+
+ return ret;
+}
+
+static int
+setup_out_fd(const char *output, const char *outfd)
+{
+ int fd = -1;
+ int flags;
+
+ assert(!(output && outfd));
+
+ if (output) {
+ if (strcmp(output, "-") == 0) {
+ fd = STDOUT_FILENO;
+ } else {
+ fd = open(output,
+ O_WRONLY | O_APPEND | O_CREAT, 0644);
+ if (fd < 0) {
+ fprintf(stderr,
+ "Error: opening file '%s' failed: %m\n",
+ output);
+ }
+ return fd;
+ }
+ } else if (outfd) {
+ fd = atoi(outfd);
+ } else {
+ fd = STDOUT_FILENO;
+ }
+
+ flags = fcntl(fd, F_GETFL);
+ if (flags == -1) {
+ fprintf(stderr,
+ "Error: cannot use file descriptor %d: %m\n", fd);
+ return -1;
+ }
+
+ if ((flags & O_ACCMODE) != O_WRONLY &&
+ (flags & O_ACCMODE) != O_RDWR) {
+ fprintf(stderr,
+ "Error: file descriptor %d is not writable.\n", fd);
+ return -1;
+ }
+
+ return fd;
+}
+
+static void
+print_help(void)
+{
+ fprintf(stderr,
+ "Usage: weston-debug [options] [names]\n"
+ "Where options may be:\n"
+ " -h, --help\n"
+ " This help text, and exit with success.\n"
+ " -l, --list\n"
+ " Print a list of available debug streams to stderr.\n"
+ " -a, --all-streams\n"
+ " Bind to all available streams.\n"
+ " -o FILE, --output FILE\n"
+ " Direct output to file named FILE. Use - for stdout.\n"
+ " Stdout is the default. Mutually exclusive with -f.\n"
+ " -f FD, --outfd FD\n"
+ " Direct output to the file descriptor FD.\n"
+ " Stdout (1) is the default. Mutually exclusive with -o.\n"
+ "Names are whatever debug stream names the compositor supports.\n"
+ );
+}
+
+static int
+parse_cmdline(struct debug_app *app, int argc, char **argv)
+{
+ static const struct option opts[] = {
+ { "help", no_argument, NULL, 'h' },
+ { "list", no_argument, NULL, 'l' },
+ { "all-streams", no_argument, NULL, 'a' },
+ { "output", required_argument, NULL, 'o' },
+ { "outfd", required_argument, NULL, 'f' },
+ { 0 }
+ };
+ static const char optstr[] = "hlao:f:";
+ int c;
+ bool failed = false;
+
+ while (1) {
+ c = getopt_long(argc, argv, optstr, opts, NULL);
+ if (c == -1)
+ break;
+
+ switch (c) {
+ case 'h':
+ app->opt.help = true;
+ break;
+ case 'l':
+ app->opt.list = true;
+ break;
+ case 'a':
+ app->opt.bind_all = true;
+ break;
+ case 'o':
+ free(app->opt.output);
+ app->opt.output = strdup(optarg);
+ break;
+ case 'f':
+ free(app->opt.outfd);
+ app->opt.outfd = strdup(optarg);
+ break;
+ case '?':
+ failed = true;
+ break;
+ default:
+ fprintf(stderr, "huh? getopt => %c (%d)\n", c, c);
+ failed = true;
+ }
+ }
+
+ if (failed)
+ return -1;
+
+ while (optind < argc) {
+ struct debug_stream *stream = stream_alloc(app, argv[optind++]);
+ stream->should_bind = true;
+ }
+
+ return 0;
+}
+
+int
+main(int argc, char **argv)
+{
+ struct debug_app app = {};
+ int ret = 0;
+
+ wl_list_init(&app.stream_list);
+ app.out_fd = -1;
+
+ if (parse_cmdline(&app, argc, argv) < 0) {
+ ret = 1;
+ goto out_parse;
+ }
+
+ if (app.opt.help) {
+ print_help();
+ goto out_parse;
+ }
+
+ if (app.opt.output && app.opt.outfd) {
+ fprintf(stderr, "Error: options --output and --outfd cannot be used simultaneously.\n");
+ ret = 1;
+ goto out_parse;
+ }
+
+ app.out_fd = setup_out_fd(app.opt.output, app.opt.outfd);
+ if (app.out_fd < 0) {
+ ret = 1;
+ goto out_parse;
+ }
+
+ app.dpy = wl_display_connect(NULL);
+ if (!app.dpy) {
+ fprintf(stderr, "Error: Could not connect to Wayland display: %m\n");
+ ret = 1;
+ goto out_parse;
+ }
+
+ app.registry = wl_display_get_registry(app.dpy);
+ wl_registry_add_listener(app.registry, &registry_listener, &app);
+ wl_display_roundtrip(app.dpy);
+
+ if (!app.debug_iface) {
+ ret = 1;
+ fprintf(stderr,
+ "The Wayland server does not support %s interface.\n",
+ weston_debug_v1_interface.name);
+ goto out_conn;
+ }
+
+ wl_display_roundtrip(app.dpy); /* for weston_debug_v1::advertise */
+
+ if (app.opt.list)
+ list_streams(&app);
+
+ start_streams(&app);
+
+ weston_debug_v1_destroy(app.debug_iface);
+
+ while (1) {
+ struct debug_stream *stream;
+ bool empty = true;
+
+ wl_list_for_each(stream, &app.stream_list, link) {
+ if (stream->obj) {
+ empty = false;
+ break;
+ }
+ }
+
+ if (empty)
+ break;
+
+ if (wl_display_dispatch(app.dpy) < 0) {
+ ret = 1;
+ break;
+ }
+ }
+
+out_conn:
+ destroy_streams(&app);
+
+ /* Wait for server to close all files */
+ wl_display_roundtrip(app.dpy);
+
+ wl_registry_destroy(app.registry);
+ wl_display_disconnect(app.dpy);
+
+out_parse:
+ if (app.out_fd != -1)
+ close(app.out_fd);
+
+ destroy_streams(&app);
+ free(app.opt.output);
+ free(app.opt.outfd);
+
+ return ret;
+}
diff --git a/man/weston-debug.man b/man/weston-debug.man
new file mode 100644
index 000000000..3a6a17617
--- /dev/null
+++ b/man/weston-debug.man
@@ -0,0 +1,46 @@
+.TH WESTON-DEBUG 1 "2017-08-02" "Weston __version__"
+.SH NAME
+weston-debug \- a tool for getting debug messages from compositor.
+.SH SYNOPSIS
+.B weston-debug [options] [names]
+.
+.\" ***************************************************************
+.SH DESCRIPTION
+
+.B weston-debug
+is a debugging tool which uses weston_debug_v1 interface to get the
+debug messages from the compositor. The debug messages are categorized into different
+debug streams by the compositor (example: logs, proto, list, etc.,) and the compositor
+requires a file descriptor to stream the messages.
+
+This tool accepts a file name or a file desciptor (not both) and any desired debug stream
+names from the user as command line arguments and subscribes the desired streams from the
+compositor by using the zcompositor_debug_v1 interface. After the subscription, the
+compositor will start to write the debug messages to the shared file descriptor.
+
+If no file name or file descriptor argument is given, the tool will use the stdout file
+descriptor. If no debug stream name argument is given, the tool will use the the name "list"
+which results the names of all the supported debug streams by the compositor.
+
+.
+.\" ***************************************************************
+.SH OPTIONS
+.
+.B weston-debug
+accepts the following command line options.
+.TP
+. B \-h, \-\-help
+Print the help text and exit with success.
+.TP
+. B \-o FILE, \-\-output FILE
+Direct output to file named FILE. Use - for stdout.
+Stdout is the default. Mutually exclusive with -f.
+.TP
+. B \-f FD, \-\-outfd FD
+Direct output to the file descriptor FD.
+Stdout (1) is the default. Mutually exclusive with -o.
+.TP
+.B [names]
+Names are whatever debug stream names the compositor supports. If none
+are given, the name "list" is used, to which the compositor should reply
+with a list of all supported names.
--
2.17.1
Ucan, Emre (ADITG/ESB)
2018-07-26 14:26:34 UTC
Permalink
Reviewed-by: Emre Ucan <***@de.adit-jv.com>

Best regards

Emre Ucan
Engineering Software Base (ADITG/ESB)

Tel. +49 5121 49 6937
-----Original Message-----
From: wayland-devel [mailto:wayland-devel-
Sent: Freitag, 20. Juli 2018 21:07
Subject: [PATCH weston v5 05/14] clients: add weston-debug
A tool for accessing the zcompositor_debug_v1 interface features.
Installed along weston-info, because it should be potentially useful for
people running libweston-based compositors.
Added a man page for weston-debug client
[Pekka: fixed 'missing braces aroudn initializer' warning]
Add --list and --all arguments, using interface advertisement.
---
Makefile.am | 16 +-
clients/weston-debug.c | 453
+++++++++++++++++++++++++++++++++++++++++
man/weston-debug.man | 46 +++++
3 files changed, 513 insertions(+), 2 deletions(-)
create mode 100644 clients/weston-debug.c
create mode 100644 man/weston-debug.man
diff --git a/Makefile.am b/Makefile.am
index c2d9048b3..04381e0f7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -531,7 +531,7 @@ spring_tool_SOURCES = \
if BUILD_CLIENTS
-bin_PROGRAMS += weston-terminal weston-info
+bin_PROGRAMS += weston-terminal weston-info weston-debug
libexec_PROGRAMS += \
weston-desktop-shell \
@@ -862,6 +862,15 @@ nodist_weston_info_SOURCES =
\
weston_info_LDADD = $(WESTON_INFO_LIBS) libshared.la
weston_info_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
+weston_debug_SOURCES = \
+ clients/weston-debug.c \
+ shared/helpers.h
+nodist_weston_debug_SOURCES = \
+ protocol/weston-debug-protocol.c \
+ protocol/weston-debug-client-protocol.h
+weston_debug_LDADD = $(WESTON_INFO_LIBS) libshared.la
+weston_debug_CFLAGS = $(AM_CFLAGS) $(CLIENT_CFLAGS)
+
weston_desktop_shell_SOURCES = \
clients/desktop-shell.c \
shared/helpers.h
@@ -898,6 +907,8 @@ BUILT_SOURCES +=
\
protocol/weston-screenshooter-client-protocol.h
\
protocol/weston-touch-calibration-protocol.c \
protocol/weston-touch-calibration-client-protocol.h \
+ protocol/weston-debug-protocol.c \
+ protocol/weston-debug-client-protocol.h
\
protocol/text-cursor-position-client-protocol.h \
protocol/text-cursor-position-protocol.c \
protocol/text-input-unstable-v1-protocol.c \
@@ -1595,7 +1606,7 @@ surface_screenshot_la_SOURCES = tests/surface-
screenshot.c
# Documentation
#
-man_MANS = weston.1 weston.ini.5
+man_MANS = weston.1 weston.ini.5 weston-debug.1
if ENABLE_DRM_COMPOSITOR
man_MANS += weston-drm.7
@@ -1623,6 +1634,7 @@ SUFFIXES = .1 .5 .7 .man
EXTRA_DIST += \
doc/calibration-helper.bash \
man/weston.man \
+ man/weston-debug.man \
man/weston-drm.man \
man/weston-rdp.man \
man/weston.ini.man
diff --git a/clients/weston-debug.c b/clients/weston-debug.c
new file mode 100644
index 000000000..59dacd269
--- /dev/null
+++ b/clients/weston-debug.c
@@ -0,0 +1,453 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include <errno.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include <wayland-client.h>
+
+#include "shared/helpers.h"
+#include "shared/zalloc.h"
+#include "weston-debug-client-protocol.h"
+
+struct debug_app {
+ struct {
+ bool help;
+ bool list;
+ bool bind_all;
+ char *output;
+ char *outfd;
+ } opt;
+
+ int out_fd;
+ struct wl_display *dpy;
+ struct wl_registry *registry;
+ struct weston_debug_v1 *debug_iface;
+ struct wl_list stream_list;
+};
+
+struct debug_stream {
+ struct wl_list link;
+ bool should_bind;
+ char *name;
+ struct weston_debug_stream_v1 *obj;
+};
+
+static struct debug_stream *
+stream_alloc(struct debug_app *app, const char *name)
+{
+ struct debug_stream *stream;
+
+ stream = zalloc(sizeof *stream);
+ if (!stream)
+ return NULL;
+
+ stream->name = strdup(name);
+ if (!stream->name) {
+ free(stream);
+ return NULL;
+ }
+
+ stream->should_bind = app->opt.bind_all;
+ wl_list_insert(app->stream_list.prev, &stream->link);
+
+ return stream;
+}
+
+static struct debug_stream *
+stream_find(struct debug_app *app, const char *name)
+{
+ struct debug_stream *stream;
+
+ wl_list_for_each(stream, &app->stream_list, link) {
+ if (strcmp(stream->name, name) == 0)
+ return stream;
+ }
+
+ return stream_alloc(app, name);
+}
+
+static void
+stream_destroy(struct debug_stream *stream)
+{
+ if (stream->obj)
+ weston_debug_stream_v1_destroy(stream->obj);
+
+ wl_list_remove(&stream->link);
+ free(stream->name);
+ free(stream);
+}
+
+static void
+destroy_streams(struct debug_app *app)
+{
+ struct debug_stream *stream;
+ struct debug_stream *tmp;
+
+ wl_list_for_each_safe(stream, tmp, &app->stream_list, link)
+ stream_destroy(stream);
+}
+
+static void
+debug_advertise(void *data, struct weston_debug_v1 *debug, const char *name)
+{
+ struct debug_app *app = data;
+ (void) stream_find(app, name);
+}
+
+static const struct weston_debug_v1_listener debug_listener = {
+ debug_advertise,
+};
+
+static void
+global_handler(void *data, struct wl_registry *registry, uint32_t id,
+ const char *interface, uint32_t version)
+{
+ struct debug_app *app = data;
+ uint32_t myver;
+
+ assert(app->registry == registry);
+
+ if (!strcmp(interface, weston_debug_v1_interface.name)) {
+ if (app->debug_iface)
+ return;
+
+ myver = MIN(1, version);
+ app->debug_iface =
+ wl_registry_bind(registry, id,
+ &weston_debug_v1_interface,
myver);
+ weston_debug_v1_add_listener(app->debug_iface,
&debug_listener,
+ app);
+ }
+}
+
+static void
+global_remove_handler(void *data, struct wl_registry *registry, uint32_t name)
+{
+}
+
+static const struct wl_registry_listener registry_listener = {
+ global_handler,
+ global_remove_handler
+};
+
+static void
+handle_stream_complete(void *data, struct weston_debug_stream_v1 *obj)
+{
+ struct debug_stream *stream = data;
+
+ assert(stream->obj == obj);
+
+ stream_destroy(stream);
+}
+
+static void
+handle_stream_failure(void *data, struct weston_debug_stream_v1 *obj,
+ const char *msg)
+{
+ struct debug_stream *stream = data;
+
+ assert(stream->obj == obj);
+
+ fprintf(stderr, "Debug stream '%s' aborted: %s\n", stream->name, msg);
+
+ stream_destroy(stream);
+}
+
+static const struct weston_debug_stream_v1_listener stream_listener = {
+ handle_stream_complete,
+ handle_stream_failure
+};
+
+static void
+start_streams(struct debug_app *app)
+{
+ struct debug_stream *stream;
+
+ wl_list_for_each(stream, &app->stream_list, link) {
+ if (!stream->should_bind)
+ continue;
+
+ stream->obj = weston_debug_v1_subscribe(app-
Post by Daniel Stone
debug_iface,
+ stream->name,
+ app->out_fd);
+ weston_debug_stream_v1_add_listener(stream->obj,
+ &stream_listener, stream);
+ }
+}
+
+/* Returns true if execution should continue, false to exit */
+static bool
+list_streams(struct debug_app *app)
+{
+ struct debug_stream *stream;
+ bool ret = false;
+
+ fprintf(stderr, "Available debug streams:\n");
+
+ wl_list_for_each(stream, &app->stream_list, link) {
+ if (stream->should_bind)
+ ret = true;
+ fprintf(stderr, " %s [will %sbind]\n", stream->name,
+ (stream->should_bind) ? "" : "not ");
+ }
+
+ return ret;
+}
+
+static int
+setup_out_fd(const char *output, const char *outfd)
+{
+ int fd = -1;
+ int flags;
+
+ assert(!(output && outfd));
+
+ if (output) {
+ if (strcmp(output, "-") == 0) {
+ fd = STDOUT_FILENO;
+ } else {
+ fd = open(output,
+ O_WRONLY | O_APPEND | O_CREAT, 0644);
+ if (fd < 0) {
+ fprintf(stderr,
+ "Error: opening file '%s' failed: %m\n",
+ output);
+ }
+ return fd;
+ }
+ } else if (outfd) {
+ fd = atoi(outfd);
+ } else {
+ fd = STDOUT_FILENO;
+ }
+
+ flags = fcntl(fd, F_GETFL);
+ if (flags == -1) {
+ fprintf(stderr,
+ "Error: cannot use file descriptor %d: %m\n", fd);
+ return -1;
+ }
+
+ if ((flags & O_ACCMODE) != O_WRONLY &&
+ (flags & O_ACCMODE) != O_RDWR) {
+ fprintf(stderr,
+ "Error: file descriptor %d is not writable.\n", fd);
+ return -1;
+ }
+
+ return fd;
+}
+
+static void
+print_help(void)
+{
+ fprintf(stderr,
+ "Usage: weston-debug [options] [names]\n"
+ "Where options may be:\n"
+ " -h, --help\n"
+ " This help text, and exit with success.\n"
+ " -l, --list\n"
+ " Print a list of available debug streams to stderr.\n"
+ " -a, --all-streams\n"
+ " Bind to all available streams.\n"
+ " -o FILE, --output FILE\n"
+ " Direct output to file named FILE. Use - for stdout.\n"
+ " Stdout is the default. Mutually exclusive with -f.\n"
+ " -f FD, --outfd FD\n"
+ " Direct output to the file descriptor FD.\n"
+ " Stdout (1) is the default. Mutually exclusive with -o.\n"
+ "Names are whatever debug stream names the compositor
supports.\n"
+ );
+}
+
+static int
+parse_cmdline(struct debug_app *app, int argc, char **argv)
+{
+ static const struct option opts[] = {
+ { "help", no_argument, NULL, 'h' },
+ { "list", no_argument, NULL, 'l' },
+ { "all-streams", no_argument, NULL, 'a' },
+ { "output", required_argument, NULL, 'o' },
+ { "outfd", required_argument, NULL, 'f' },
+ { 0 }
+ };
+ static const char optstr[] = "hlao:f:";
+ int c;
+ bool failed = false;
+
+ while (1) {
+ c = getopt_long(argc, argv, optstr, opts, NULL);
+ if (c == -1)
+ break;
+
+ switch (c) {
+ app->opt.help = true;
+ break;
+ app->opt.list = true;
+ break;
+ app->opt.bind_all = true;
+ break;
+ free(app->opt.output);
+ app->opt.output = strdup(optarg);
+ break;
+ free(app->opt.outfd);
+ app->opt.outfd = strdup(optarg);
+ break;
+ failed = true;
+ break;
+ fprintf(stderr, "huh? getopt => %c (%d)\n", c, c);
+ failed = true;
+ }
+ }
+
+ if (failed)
+ return -1;
+
+ while (optind < argc) {
+ struct debug_stream *stream = stream_alloc(app,
argv[optind++]);
+ stream->should_bind = true;
+ }
+
+ return 0;
+}
+
+int
+main(int argc, char **argv)
+{
+ struct debug_app app = {};
+ int ret = 0;
+
+ wl_list_init(&app.stream_list);
+ app.out_fd = -1;
+
+ if (parse_cmdline(&app, argc, argv) < 0) {
+ ret = 1;
+ goto out_parse;
+ }
+
+ if (app.opt.help) {
+ print_help();
+ goto out_parse;
+ }
+
+ if (app.opt.output && app.opt.outfd) {
+ fprintf(stderr, "Error: options --output and --outfd cannot be used simultaneously.\n");
+ ret = 1;
+ goto out_parse;
+ }
+
+ app.out_fd = setup_out_fd(app.opt.output, app.opt.outfd);
+ if (app.out_fd < 0) {
+ ret = 1;
+ goto out_parse;
+ }
+
+ app.dpy = wl_display_connect(NULL);
+ if (!app.dpy) {
+ fprintf(stderr, "Error: Could not connect to Wayland
display: %m\n");
+ ret = 1;
+ goto out_parse;
+ }
+
+ app.registry = wl_display_get_registry(app.dpy);
+ wl_registry_add_listener(app.registry, &registry_listener, &app);
+ wl_display_roundtrip(app.dpy);
+
+ if (!app.debug_iface) {
+ ret = 1;
+ fprintf(stderr,
+ "The Wayland server does not support %s
interface.\n",
+ weston_debug_v1_interface.name);
+ goto out_conn;
+ }
+
+ wl_display_roundtrip(app.dpy); /* for weston_debug_v1::advertise */
+
+ if (app.opt.list)
+ list_streams(&app);
+
+ start_streams(&app);
+
+ weston_debug_v1_destroy(app.debug_iface);
+
+ while (1) {
+ struct debug_stream *stream;
+ bool empty = true;
+
+ wl_list_for_each(stream, &app.stream_list, link) {
+ if (stream->obj) {
+ empty = false;
+ break;
+ }
+ }
+
+ if (empty)
+ break;
+
+ if (wl_display_dispatch(app.dpy) < 0) {
+ ret = 1;
+ break;
+ }
+ }
+
+ destroy_streams(&app);
+
+ /* Wait for server to close all files */
+ wl_display_roundtrip(app.dpy);
+
+ wl_registry_destroy(app.registry);
+ wl_display_disconnect(app.dpy);
+
+ if (app.out_fd != -1)
+ close(app.out_fd);
+
+ destroy_streams(&app);
+ free(app.opt.output);
+ free(app.opt.outfd);
+
+ return ret;
+}
diff --git a/man/weston-debug.man b/man/weston-debug.man
new file mode 100644
index 000000000..3a6a17617
--- /dev/null
+++ b/man/weston-debug.man
@@ -0,0 +1,46 @@
+.TH WESTON-DEBUG 1 "2017-08-02" "Weston __version__"
+.SH NAME
+weston-debug \- a tool for getting debug messages from compositor.
+.SH SYNOPSIS
+.B weston-debug [options] [names]
+.
+.\"
**********************************************************
*****
+.SH DESCRIPTION
+
+.B weston-debug
+is a debugging tool which uses weston_debug_v1 interface to get the
+debug messages from the compositor. The debug messages are
categorized into different
+debug streams by the compositor (example: logs, proto, list, etc.,) and the compositor
+requires a file descriptor to stream the messages.
+
+This tool accepts a file name or a file desciptor (not both) and any desired debug stream
+names from the user as command line arguments and subscribes the desired streams from the
+compositor by using the zcompositor_debug_v1 interface. After the subscription, the
+compositor will start to write the debug messages to the shared file descriptor.
+
+If no file name or file descriptor argument is given, the tool will use the stdout file
+descriptor. If no debug stream name argument is given, the tool will use the
the name "list"
+which results the names of all the supported debug streams by the compositor.
+
+.
+.\"
**********************************************************
*****
+.SH OPTIONS
+.
+.B weston-debug
+accepts the following command line options.
+.TP
+. B \-h, \-\-help
+Print the help text and exit with success.
+.TP
+. B \-o FILE, \-\-output FILE
+Direct output to file named FILE. Use - for stdout.
+Stdout is the default. Mutually exclusive with -f.
+.TP
+. B \-f FD, \-\-outfd FD
+Direct output to the file descriptor FD.
+Stdout (1) is the default. Mutually exclusive with -o.
+.TP
+.B [names]
+Names are whatever debug stream names the compositor supports. If none
+are given, the name "list" is used, to which the compositor should reply
+with a list of all supported names.
--
2.17.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-08-06 10:50:40 UTC
Permalink
On Fri, 20 Jul 2018 20:03:26 +0100
Post by Daniel Stone
A tool for accessing the zcompositor_debug_v1 interface features.
Installed along weston-info, because it should be potentially useful for
people running libweston-based compositors.
Added a man page for weston-debug client
[Pekka: fixed 'missing braces aroudn initializer' warning]
Add --list and --all arguments, using interface advertisement.
---
Makefile.am | 16 +-
clients/weston-debug.c | 453 +++++++++++++++++++++++++++++++++++++++++
man/weston-debug.man | 46 +++++
3 files changed, 513 insertions(+), 2 deletions(-)
create mode 100644 clients/weston-debug.c
create mode 100644 man/weston-debug.man
...
Post by Daniel Stone
diff --git a/clients/weston-debug.c b/clients/weston-debug.c
new file mode 100644
index 000000000..59dacd269
--- /dev/null
+++ b/clients/weston-debug.c
@@ -0,0 +1,453 @@
+/*
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "config.h"
+
+#include <errno.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <assert.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+#include <wayland-client.h>
+
+#include "shared/helpers.h"
+#include "shared/zalloc.h"
+#include "weston-debug-client-protocol.h"
+
+struct debug_app {
+ struct {
+ bool help;
+ bool list;
+ bool bind_all;
+ char *output;
+ char *outfd;
+ } opt;
+
+ int out_fd;
+ struct wl_display *dpy;
+ struct wl_registry *registry;
+ struct weston_debug_v1 *debug_iface;
+ struct wl_list stream_list;
+};
+
+struct debug_stream {
+ struct wl_list link;
+ bool should_bind;
+ char *name;
+ struct weston_debug_stream_v1 *obj;
+};
...
Post by Daniel Stone
+static void
+debug_advertise(void *data, struct weston_debug_v1 *debug, const char *name)
+{
+ struct debug_app *app = data;
Add empty line.
Post by Daniel Stone
+ (void) stream_find(app, name);
+}
+
+static const struct weston_debug_v1_listener debug_listener = {
+ debug_advertise,
+};
...
Post by Daniel Stone
+static int
+parse_cmdline(struct debug_app *app, int argc, char **argv)
+{
+ static const struct option opts[] = {
+ { "help", no_argument, NULL, 'h' },
+ { "list", no_argument, NULL, 'l' },
+ { "all-streams", no_argument, NULL, 'a' },
+ { "output", required_argument, NULL, 'o' },
+ { "outfd", required_argument, NULL, 'f' },
+ { 0 }
+ };
+ static const char optstr[] = "hlao:f:";
+ int c;
+ bool failed = false;
+
+ while (1) {
+ c = getopt_long(argc, argv, optstr, opts, NULL);
+ if (c == -1)
+ break;
+
+ switch (c) {
+ app->opt.help = true;
+ break;
+ app->opt.list = true;
+ break;
+ app->opt.bind_all = true;
+ break;
+ free(app->opt.output);
+ app->opt.output = strdup(optarg);
+ break;
+ free(app->opt.outfd);
+ app->opt.outfd = strdup(optarg);
+ break;
+ failed = true;
+ break;
+ fprintf(stderr, "huh? getopt => %c (%d)\n", c, c);
+ failed = true;
+ }
+ }
+
+ if (failed)
+ return -1;
+
+ while (optind < argc) {
+ struct debug_stream *stream = stream_alloc(app, argv[optind++]);
Add empty line.
Post by Daniel Stone
+ stream->should_bind = true;
+ }
+
+ return 0;
+}
When I tried the tool, I found this example:

$ weston-debug --list akjdakjdhask
Available debug streams:
akjdakjdhask [will bind]
scene-graph [will not bind]
log [will not bind]
proto [will not bind]
xwm-wm-x11 [will not bind]
Debug stream 'akjdakjdhask' aborted: Debug stream name 'akjdakjdhask' is unknown.

It is confusing to see 'akjdakjdhask' listed as available and then said
to be unknown.
Post by Daniel Stone
diff --git a/man/weston-debug.man b/man/weston-debug.man
new file mode 100644
index 000000000..3a6a17617
--- /dev/null
+++ b/man/weston-debug.man
@@ -0,0 +1,46 @@
+.TH WESTON-DEBUG 1 "2017-08-02" "Weston __version__"
+.SH NAME
+weston-debug \- a tool for getting debug messages from compositor.
+.SH SYNOPSIS
+.B weston-debug [options] [names]
+.
+.\" ***************************************************************
+.SH DESCRIPTION
+
+.B weston-debug
+is a debugging tool which uses weston_debug_v1 interface to get the
+debug messages from the compositor. The debug messages are categorized into different
+debug streams by the compositor (example: logs, proto, list, etc.,) and the compositor
+requires a file descriptor to stream the messages.
+
+This tool accepts a file name or a file desciptor (not both) and any desired debug stream
+names from the user as command line arguments and subscribes the desired streams from the
+compositor by using the zcompositor_debug_v1 interface. After the subscription, the
It is called weston_debug_v1 in this iteration.
Post by Daniel Stone
+compositor will start to write the debug messages to the shared file descriptor.
+
+If no file name or file descriptor argument is given, the tool will use the stdout file
+descriptor. If no debug stream name argument is given, the tool will use the the name "list"
+which results the names of all the supported debug streams by the compositor.
There is no stream named "list" in the compositor anymore.
Post by Daniel Stone
+
+.
+.\" ***************************************************************
+.SH OPTIONS
+.
+.B weston-debug
+accepts the following command line options.
+.TP
+. B \-h, \-\-help
+Print the help text and exit with success.
--list and --all-streams arguments missing.
Post by Daniel Stone
+.TP
+. B \-o FILE, \-\-output FILE
+Direct output to file named FILE. Use - for stdout.
+Stdout is the default. Mutually exclusive with -f.
+.TP
+. B \-f FD, \-\-outfd FD
+Direct output to the file descriptor FD.
+Stdout (1) is the default. Mutually exclusive with -o.
+.TP
+.B [names]
+Names are whatever debug stream names the compositor supports. If none
+are given, the name "list" is used, to which the compositor should reply
+with a list of all supported names.
In the code, if no options are given, the tool now exits without
printing anything. How about defaulting to the --list behaviour?

Not sure how understandable the "[will not bind]" messages are when one
simply does 'weston-debug --list'. It sounds a bit like "it's there but
you can't have it". Maybe make it: bind ? " [subscribed]" : "".


Thanks,
pq
Daniel Stone
2018-07-20 19:03:31 UTC
Permalink
From: Pekka Paalanen <***@iki.fi>

This is better than running Weston with WAYLAND_DEBUG=server:
- It is enabled on demand, no unnecessary flooding and no need to
restart the compositor if debug was enabled.
- It prints client pointers so that messages with different clients can
be seen apart.

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

parse and print message arguments in protocol_log_fn

Signed-off-by: Maniraj Devadoss <***@in.bosch.com>
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>
Reviewed-by: Daniel Stone <***@collabora.com>
---
compositor/main.c | 129 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 127 insertions(+), 2 deletions(-)

diff --git a/compositor/main.c b/compositor/main.c
index eaf4cf381..3cb791384 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -120,6 +120,7 @@ struct wet_compositor {

static FILE *weston_logfile = NULL;
static struct weston_debug_scope *log_scope;
+static struct weston_debug_scope *protocol_scope;

static int cached_tm_mday = -1;

@@ -214,6 +215,116 @@ vlog_continue(const char *fmt, va_list argp)
return vfprintf(weston_logfile, fmt, argp);
}

+static const char *
+get_next_argument(const char *signature, char* type)
+{
+ for(; *signature; ++signature) {
+ switch(*signature) {
+ case 'i':
+ case 'u':
+ case 'f':
+ case 's':
+ case 'o':
+ case 'n':
+ case 'a':
+ case 'h':
+ *type = *signature;
+ return signature + 1;
+ }
+ }
+ *type = '\0';
+ return signature;
+}
+
+static void
+protocol_log_fn(void *user_data,
+ enum wl_protocol_logger_type direction,
+ const struct wl_protocol_logger_message *message)
+{
+ FILE *fp;
+ char *logstr;
+ size_t logsize;
+ char timestr[128];
+ struct wl_resource *res = message->resource;
+ const char *signature = message->message->signature;
+ int i;
+ char type;
+
+ if (!weston_debug_scope_is_enabled(protocol_scope))
+ return;
+
+ fp = open_memstream(&logstr, &logsize);
+ if (!fp)
+ return;
+
+ weston_debug_scope_timestamp(protocol_scope,
+ timestr, sizeof timestr);
+ fprintf(fp, "%s ", timestr);
+ fprintf(fp, "client %p %s ", wl_resource_get_client(res),
+ direction == WL_PROTOCOL_LOGGER_REQUEST ? "rq" : "ev");
+ fprintf(fp, "%s@%u.%s(",
+ wl_resource_get_class(res),
+ wl_resource_get_id(res),
+ message->message->name);
+
+ for (i = 0; i < message->arguments_count; i++) {
+ signature = get_next_argument(signature, &type);
+
+ if (i > 0)
+ fprintf(fp, ", ");
+
+ switch (type) {
+ case 'u':
+ fprintf(fp, "%u", message->arguments[i].u);
+ break;
+ case 'i':
+ fprintf(fp, "%d", message->arguments[i].i);
+ break;
+ case 'f':
+ fprintf(fp, "%f",
+ wl_fixed_to_double(message->arguments[i].f));
+ break;
+ case 's':
+ fprintf(fp, "\"%s\"", message->arguments[i].s);
+ break;
+ case 'o':
+ if (message->arguments[i].o) {
+ struct wl_resource* resource;
+ resource = (struct wl_resource*) message->arguments[i].o;
+ fprintf(fp, "%s@%u",
+ wl_resource_get_class(resource),
+ wl_resource_get_id(resource));
+ }
+ else
+ fprintf(fp, "nil");
+ break;
+ case 'n':
+ fprintf(fp, "new id %s@",
+ (message->message->types[i]) ?
+ message->message->types[i]->name :
+ "[unknown]");
+ if (message->arguments[i].n != 0)
+ fprintf(fp, "%u", message->arguments[i].n);
+ else
+ fprintf(fp, "nil");
+ break;
+ case 'a':
+ fprintf(fp, "array");
+ break;
+ case 'h':
+ fprintf(fp, "fd %d", message->arguments[i].h);
+ break;
+ }
+ }
+
+ fprintf(fp, ")\n");
+
+ if (fclose(fp) == 0)
+ weston_debug_scope_write(protocol_scope, logstr, logsize);
+
+ free(logstr);
+}
+
static struct wl_list child_process_list;
static struct weston_compositor *segv_compositor;

@@ -2406,6 +2517,7 @@ int main(int argc, char *argv[])
struct wet_compositor wet = { 0 };
int require_input;
int32_t wait_for_debugger = 0;
+ struct wl_protocol_logger *protologger = NULL;

const struct weston_option core_options[] = {
{ WESTON_OPTION_STRING, "backend", 'B', &backend },
@@ -2510,9 +2622,18 @@ int main(int argc, char *argv[])

log_scope = weston_compositor_add_debug_scope(wet.compositor, "log",
"Weston and Wayland log\n", NULL, NULL);
-
- if (debug_protocol)
+ protocol_scope =
+ weston_compositor_add_debug_scope(wet.compositor,
+ "proto",
+ "Wayland protocol dump for all clients.\n",
+ NULL, NULL);
+
+ if (debug_protocol) {
+ protologger = wl_display_add_protocol_logger(display,
+ protocol_log_fn,
+ NULL);
weston_compositor_enable_debug_protocol(wet.compositor);
+ }

if (weston_compositor_init_config(wet.compositor, config) < 0)
goto out;
@@ -2623,6 +2744,10 @@ out:
/* free(NULL) is valid, and it won't be NULL if it's used */
free(wet.parsed_options);

+ if (protologger)
+ wl_protocol_logger_destroy(protologger);
+
+ weston_debug_scope_destroy(protocol_scope);
weston_debug_scope_destroy(log_scope);
weston_compositor_destroy(wet.compositor);
--
2.17.1
Ucan, Emre (ADITG/ESB)
2018-07-30 09:17:27 UTC
Permalink
Hi Daniel,

We found an issue with this patch. I am adding a patch to this email to fix this issue. Please check:

Subject: [PATCH] main: copy va_list for second use

we are using va_list once for debug protocol and
once for local logging to stdout. After first use
the list is invalidated. Therefore, we have to copy
it for the second use.

Signed-off-by: Emre Ucan <***@de.adit-jv.com>
---
compositor/main.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/compositor/main.c b/compositor/main.c
index 3333147..7975af0 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -193,6 +193,9 @@ vlog(const char *fmt, va_list ap)
{
int l;
char timestr[128];
+ va_list ap2;
+
+ va_copy(ap2, ap);

if (weston_debug_scope_is_enabled(log_scope)) {
weston_debug_scope_printf(log_scope, "%s ",
@@ -202,7 +205,8 @@ vlog(const char *fmt, va_list ap)
}

l = weston_log_timestamp();
- l += vfprintf(weston_logfile, fmt, ap);
+ l += vfprintf(weston_logfile, fmt, ap2);
+ va_end(ap2);

return l;
}
--
2.7.4
-----Original Message-----
From: wayland-devel [mailto:wayland-devel-
Sent: Freitag, 20. Juli 2018 21:07
Subject: [PATCH weston v5 10/14] compositor: protocol logger
- It is enabled on demand, no unnecessary flooding and no need to
restart the compositor if debug was enabled.
- It prints client pointers so that messages with different clients can
be seen apart.
parse and print message arguments in protocol_log_fn
---
compositor/main.c | 129
+++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 127 insertions(+), 2 deletions(-)
diff --git a/compositor/main.c b/compositor/main.c
index eaf4cf381..3cb791384 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -120,6 +120,7 @@ struct wet_compositor {
static FILE *weston_logfile = NULL;
static struct weston_debug_scope *log_scope;
+static struct weston_debug_scope *protocol_scope;
static int cached_tm_mday = -1;
@@ -214,6 +215,116 @@ vlog_continue(const char *fmt, va_list argp)
return vfprintf(weston_logfile, fmt, argp);
}
+static const char *
+get_next_argument(const char *signature, char* type)
+{
+ for(; *signature; ++signature) {
+ switch(*signature) {
+ *type = *signature;
+ return signature + 1;
+ }
+ }
+ *type = '\0';
+ return signature;
+}
+
+static void
+protocol_log_fn(void *user_data,
+ enum wl_protocol_logger_type direction,
+ const struct wl_protocol_logger_message *message)
+{
+ FILE *fp;
+ char *logstr;
+ size_t logsize;
+ char timestr[128];
+ struct wl_resource *res = message->resource;
+ const char *signature = message->message->signature;
+ int i;
+ char type;
+
+ if (!weston_debug_scope_is_enabled(protocol_scope))
+ return;
+
+ fp = open_memstream(&logstr, &logsize);
+ if (!fp)
+ return;
+
+ weston_debug_scope_timestamp(protocol_scope,
+ timestr, sizeof timestr);
+ fprintf(fp, "%s ", timestr);
+ fprintf(fp, "client %p %s ", wl_resource_get_client(res),
"ev");
+ wl_resource_get_class(res),
+ wl_resource_get_id(res),
+ message->message->name);
+
+ for (i = 0; i < message->arguments_count; i++) {
+ signature = get_next_argument(signature, &type);
+
+ if (i > 0)
+ fprintf(fp, ", ");
+
+ switch (type) {
+ fprintf(fp, "%u", message->arguments[i].u);
+ break;
+ fprintf(fp, "%d", message->arguments[i].i);
+ break;
+ fprintf(fp, "%f",
+ wl_fixed_to_double(message-
Post by Daniel Stone
arguments[i].f));
+ break;
+ fprintf(fp, "\"%s\"", message->arguments[i].s);
+ break;
+ if (message->arguments[i].o) {
+ struct wl_resource* resource;
+ resource = (struct wl_resource*) message-
Post by Daniel Stone
arguments[i].o;
+ wl_resource_get_class(resource),
+ wl_resource_get_id(resource));
+ }
+ else
+ fprintf(fp, "nil");
+ break;
+ (message->message->types[i]) ?
+ "[unknown]");
+ if (message->arguments[i].n != 0)
+ fprintf(fp, "%u", message->arguments[i].n);
+ else
+ fprintf(fp, "nil");
+ break;
+ fprintf(fp, "array");
+ break;
+ fprintf(fp, "fd %d", message->arguments[i].h);
+ break;
+ }
+ }
+
+ fprintf(fp, ")\n");
+
+ if (fclose(fp) == 0)
+ weston_debug_scope_write(protocol_scope, logstr,
logsize);
+
+ free(logstr);
+}
+
static struct wl_list child_process_list;
static struct weston_compositor *segv_compositor;
@@ -2406,6 +2517,7 @@ int main(int argc, char *argv[])
struct wet_compositor wet = { 0 };
int require_input;
int32_t wait_for_debugger = 0;
+ struct wl_protocol_logger *protologger = NULL;
const struct weston_option core_options[] = {
{ WESTON_OPTION_STRING, "backend", 'B', &backend },
@@ -2510,9 +2622,18 @@ int main(int argc, char *argv[])
log_scope =
weston_compositor_add_debug_scope(wet.compositor, "log",
"Weston and Wayland log\n", NULL, NULL);
-
- if (debug_protocol)
+ protocol_scope =
+ weston_compositor_add_debug_scope(wet.compositor,
+ "proto",
+ "Wayland protocol dump for all clients.\n",
+ NULL, NULL);
+
+ if (debug_protocol) {
+ protologger = wl_display_add_protocol_logger(display,
+ protocol_log_fn,
+ NULL);
weston_compositor_enable_debug_protocol(wet.compositor);
+ }
if (weston_compositor_init_config(wet.compositor, config) < 0)
goto out;
/* free(NULL) is valid, and it won't be NULL if it's used */
free(wet.parsed_options);
+ if (protologger)
+ wl_protocol_logger_destroy(protologger);
+
+ weston_debug_scope_destroy(protocol_scope);
weston_debug_scope_destroy(log_scope);
weston_compositor_destroy(wet.compositor);
--
2.17.1
_______________________________________________
wayland-devel mailing list
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Pekka Paalanen
2018-08-06 13:03:09 UTC
Permalink
On Mon, 30 Jul 2018 09:17:27 +0000
Post by Ucan, Emre (ADITG/ESB)
Hi Daniel,
Subject: [PATCH] main: copy va_list for second use
we are using va_list once for debug protocol and
once for local logging to stdout. After first use
the list is invalidated. Therefore, we have to copy
it for the second use.
---
compositor/main.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/compositor/main.c b/compositor/main.c
index 3333147..7975af0 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -193,6 +193,9 @@ vlog(const char *fmt, va_list ap)
{
int l;
char timestr[128];
+ va_list ap2;
+
+ va_copy(ap2, ap);
if (weston_debug_scope_is_enabled(log_scope)) {
weston_debug_scope_printf(log_scope, "%s ",
@@ -202,7 +205,8 @@ vlog(const char *fmt, va_list ap)
}
l = weston_log_timestamp();
- l += vfprintf(weston_logfile, fmt, ap);
+ l += vfprintf(weston_logfile, fmt, ap2);
+ va_end(ap2);
return l;
}
Hi Emre,

nice, I wonder if this was the thing causing those crashes that IIRC
Maniraj was seeing and worked around them by having
weston_debug_scope_timestamp() return void.

Reading the manual, you are correct and this is needed.

Squashing this into the appropriate commit (patch 6) retains:
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>

custom_handler() and vlog_continue() seem to need the same fix.


Thanks,
pq
Daniel Stone
2018-07-20 19:03:28 UTC
Permalink
From: Pekka Paalanen <***@iki.fi>

Write the output of dump_property() out in one log call. When multiple
processes (weston and Xwayland) are writing to the same file, this will
keep the property dump uninterrupted by Xwayland debug prints.

This is also preparation for more development in the same direction.

Signed-off-by: Pekka Paalanen <***@iki.fi>
Signed-off-by: Maniraj Devadoss <***@in.bosch.com>
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>
Reviewed-by: Daniel Stone <***@collabora.com>
---
xwayland/window-manager.c | 58 ++++++++++++++++++++-------------------
1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index 2b3defb70..3bf323a42 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -410,20 +410,14 @@ dump_cardinal_array_elem(FILE *fp, unsigned format,
}

static void
-dump_cardinal_array(xcb_get_property_reply_t *reply)
+dump_cardinal_array(FILE *fp, xcb_get_property_reply_t *reply)
{
unsigned i = 0;
- FILE *fp;
void *arr;
char *str = NULL;
- size_t size = 0;

assert(reply->type == XCB_ATOM_CARDINAL);

- fp = open_memstream(&str, &size);
- if (!fp)
- return;
-
arr = xcb_get_property_value(reply);

fprintf(fp, "[");
@@ -432,10 +426,6 @@ dump_cardinal_array(xcb_get_property_reply_t *reply)
arr, reply->value_len, i);
fprintf(fp, "]");

- if (fclose(fp) != 0)
- return;
-
- wm_log_continue("%s\n", str);
free(str);
}

@@ -449,22 +439,29 @@ dump_property(struct weston_wm *wm,
xcb_window_t *window_value;
int width, len;
uint32_t i;
+ FILE *fp;
+ char *logstr;
+ size_t logsize;

- width = wm_log_continue("%s: ", get_atom_name(wm->conn, property));
- if (reply == NULL) {
- wm_log_continue("(no reply)\n");
+ fp = open_memstream(&logstr, &logsize);
+ if (!fp)
return;
+
+ width = fprintf(fp, "%s: ", get_atom_name(wm->conn, property));
+ if (reply == NULL) {
+ fprintf(fp, "(no reply)\n");
+ goto out;
}

- width += wm_log_continue("%s/%d, length %d (value_len %d): ",
- get_atom_name(wm->conn, reply->type),
- reply->format,
- xcb_get_property_value_length(reply),
- reply->value_len);
+ width += fprintf(fp, "%s/%d, length %d (value_len %d): ",
+ get_atom_name(wm->conn, reply->type),
+ reply->format,
+ xcb_get_property_value_length(reply),
+ reply->value_len);

if (reply->type == wm->atom.incr) {
incr_value = xcb_get_property_value(reply);
- wm_log_continue("%d\n", *incr_value);
+ fprintf(fp, "%d\n", *incr_value);
} else if (reply->type == wm->atom.utf8_string ||
reply->type == wm->atom.string) {
text_value = xcb_get_property_value(reply);
@@ -472,29 +469,34 @@ dump_property(struct weston_wm *wm,
len = 40;
else
len = reply->value_len;
- wm_log_continue("\"%.*s\"\n", len, text_value);
+ fprintf(fp, "\"%.*s\"\n", len, text_value);
} else if (reply->type == XCB_ATOM_ATOM) {
atom_value = xcb_get_property_value(reply);
for (i = 0; i < reply->value_len; i++) {
name = get_atom_name(wm->conn, atom_value[i]);
if (width + strlen(name) + 2 > 78) {
- wm_log_continue("\n ");
+ fprintf(fp, "\n ");
width = 4;
} else if (i > 0) {
- width += wm_log_continue(", ");
+ width += fprintf(fp, ", ");
}

- width += wm_log_continue("%s", name);
+ width += fprintf(fp, "%s", name);
}
- wm_log_continue("\n");
+ fprintf(fp, "\n");
} else if (reply->type == XCB_ATOM_CARDINAL) {
- dump_cardinal_array(reply);
+ dump_cardinal_array(fp, reply);
} else if (reply->type == XCB_ATOM_WINDOW && reply->format == 32) {
window_value = xcb_get_property_value(reply);
- wm_log_continue("win %u\n", *window_value);
+ fprintf(fp, "win %u\n", *window_value);
} else {
- wm_log_continue("huh?\n");
+ fprintf(fp, "huh?\n");
}
+
+out:
+ if (fclose(fp) == 0)
+ wm_log_continue("%s", logstr);
+ free(logstr);
}

static void
--
2.17.1
Daniel Stone
2018-07-20 19:03:27 UTC
Permalink
From: Pekka Paalanen <***@iki.fi>

This registers a new weston-debug scope "log" through which one can get
live log output interspersed with possible other debugging prints.

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

pass the log_scope to weston_debug_scope_timestamp API to append
the scope name to the timestamp

Signed-off-by: Maniraj Devadoss <***@in.bosch.com>
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>
Reviewed-by: Daniel Stone <***@collabora.com>
---
compositor/main.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/compositor/main.c b/compositor/main.c
index 2f34e1115..eaf4cf381 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -119,6 +119,7 @@ struct wet_compositor {
};

static FILE *weston_logfile = NULL;
+static struct weston_debug_scope *log_scope;

static int cached_tm_mday = -1;

@@ -149,9 +150,16 @@ static int weston_log_timestamp(void)
static void
custom_handler(const char *fmt, va_list arg)
{
+ char timestr[128];
+
weston_log_timestamp();
fprintf(weston_logfile, "libwayland: ");
vfprintf(weston_logfile, fmt, arg);
+
+ weston_debug_scope_printf(log_scope, "%s libwayland: ",
+ weston_debug_scope_timestamp(log_scope,
+ timestr, sizeof timestr));
+ weston_debug_scope_vprintf(log_scope, fmt, arg);
}

static void
@@ -183,6 +191,14 @@ static int
vlog(const char *fmt, va_list ap)
{
int l;
+ char timestr[128];
+
+ if (weston_debug_scope_is_enabled(log_scope)) {
+ weston_debug_scope_printf(log_scope, "%s ",
+ weston_debug_scope_timestamp(log_scope,
+ timestr, sizeof timestr));
+ weston_debug_scope_vprintf(log_scope, fmt, ap);
+ }

l = weston_log_timestamp();
l += vfprintf(weston_logfile, fmt, ap);
@@ -193,6 +209,8 @@ vlog(const char *fmt, va_list ap)
static int
vlog_continue(const char *fmt, va_list argp)
{
+ weston_debug_scope_vprintf(log_scope, fmt, argp);
+
return vfprintf(weston_logfile, fmt, argp);
}

@@ -2490,6 +2508,9 @@ int main(int argc, char *argv[])
}
segv_compositor = wet.compositor;

+ log_scope = weston_compositor_add_debug_scope(wet.compositor, "log",
+ "Weston and Wayland log\n", NULL, NULL);
+
if (debug_protocol)
weston_compositor_enable_debug_protocol(wet.compositor);

@@ -2602,6 +2623,7 @@ out:
/* free(NULL) is valid, and it won't be NULL if it's used */
free(wet.parsed_options);

+ weston_debug_scope_destroy(log_scope);
weston_compositor_destroy(wet.compositor);

out_signals:
--
2.17.1
Pekka Paalanen
2018-08-06 11:20:51 UTC
Permalink
On Fri, 20 Jul 2018 20:03:27 +0100
Post by Daniel Stone
This registers a new weston-debug scope "log" through which one can get
live log output interspersed with possible other debugging prints.
pass the log_scope to weston_debug_scope_timestamp API to append
the scope name to the timestamp
---
compositor/main.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/compositor/main.c b/compositor/main.c
index 2f34e1115..eaf4cf381 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -2490,6 +2508,9 @@ int main(int argc, char *argv[])
}
segv_compositor = wet.compositor;
+ log_scope = weston_compositor_add_debug_scope(wet.compositor, "log",
+ "Weston and Wayland log\n", NULL, NULL);
+
if (debug_protocol)
weston_compositor_enable_debug_protocol(wet.compositor);
/* free(NULL) is valid, and it won't be NULL if it's used */
free(wet.parsed_options);
+ weston_debug_scope_destroy(log_scope);
Maybe log_scope should be set to NULL again, in case
weston_compositor_destroy() logs something.
Post by Daniel Stone
weston_compositor_destroy(wet.compositor);
Otherwise still good.


Thanks,
pq
Pekka Paalanen
2018-08-07 12:15:45 UTC
Permalink
On Mon, 6 Aug 2018 14:20:51 +0300
Post by Pekka Paalanen
On Fri, 20 Jul 2018 20:03:27 +0100
Post by Daniel Stone
This registers a new weston-debug scope "log" through which one can get
live log output interspersed with possible other debugging prints.
pass the log_scope to weston_debug_scope_timestamp API to append
the scope name to the timestamp
---
compositor/main.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/compositor/main.c b/compositor/main.c
index 2f34e1115..eaf4cf381 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -2490,6 +2508,9 @@ int main(int argc, char *argv[])
}
segv_compositor = wet.compositor;
+ log_scope = weston_compositor_add_debug_scope(wet.compositor, "log",
+ "Weston and Wayland log\n", NULL, NULL);
+
if (debug_protocol)
weston_compositor_enable_debug_protocol(wet.compositor);
/* free(NULL) is valid, and it won't be NULL if it's used */
free(wet.parsed_options);
+ weston_debug_scope_destroy(log_scope);
Maybe log_scope should be set to NULL again, in case
weston_compositor_destroy() logs something.
Hi,

I may have hit that, here's a backtrace from drm-backend:

Core was generated by `weston --debug'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007fb517596657 in __strlen_avx2 () from /lib64/libc.so.6
(gdb) bt
#0 0x00007fb517596657 in __strlen_avx2 () from /lib64/libc.so.6
#1 0x00007fb51748339b in vfprintf () from /lib64/libc.so.6
#2 0x00007fb517543be9 in __vsnprintf_chk () from /lib64/libc.so.6
#3 0x00007fb517543b1d in __snprintf_chk () from /lib64/libc.so.6
#4 0x00007fb518cd5a08 in snprintf (__fmt=0x7fb518cdb6db "[%s.%03ld][%s]", __n=128,
__s=0x7ffe718ca4e0 "[2018-08-07 15:03:25.484][(\026\265\177") at /usr/include/bits/stdio2.h:64
#5 weston_debug_scope_timestamp (scope=0x558d51518770, buf=0x7ffe718ca4e0 "[2018-08-07 15:03:25.484][(\026\265\177",
len=128) at /home/pq/git/weston/libweston/weston-debug.c:717
#6 0x0000558d504f5788 in vlog (fmt=0x7ffe718ca5e0 "event2 - Video Bus: device removed\n", ap=0x7ffe718caa30)
at /home/pq/git/weston/compositor/main.c:198
#7 0x00007fb517d5fbb6 in log_msg_va (libinput=0x558d51680600, priority=LIBINPUT_LOG_PRIORITY_INFO,
format=0x7ffe718ca5e0 "event2 - Video Bus: device removed\n", args=0x7ffe718caa30) at ../src/libinput.c:266
#8 0x00007fb517d67b47 in evdev_log_msg_va (device=0x558d516ad1b0, priority=LIBINPUT_LOG_PRIORITY_INFO,
format=0x7fb517d91789 "device removed\n", args=0x7ffe718caa30) at ../src/evdev.h:680
#9 0x00007fb517d67c1f in evdev_log_msg (device=0x558d516ad1b0, priority=LIBINPUT_LOG_PRIORITY_INFO,
format=0x7fb517d91789 "device removed\n") at ../src/evdev.h:693
#10 0x00007fb517d6ce45 in evdev_device_remove (device=0x558d516ad1b0) at ../src/evdev.c:2489
#11 0x00007fb517d8ce3c in udev_input_remove_devices (input=0x558d51680600) at ../src/udev-seat.c:203
#12 0x00007fb517d8cf24 in udev_input_disable (libinput=0x558d51680600) at ../src/udev-seat.c:222
#13 0x00007fb517d65241 in libinput_suspend (libinput=0x558d51680600) at ../src/libinput.c:2821
#14 0x00007fb517d63023 in libinput_unref (libinput=0x558d51680600) at ../src/libinput.c:1752
#15 0x00007fb51652f340 in udev_input_destroy (input=***@entry=0x558d5151faf8)
at /home/pq/git/weston/libweston/libinput-seat.c:373
#16 0x00007fb516529607 in drm_destroy (ec=0x558d51517f30) at /home/pq/git/weston/libweston/compositor-drm.c:6429
#17 0x00007fb518cc4a59 in weston_compositor_destroy (compositor=0x558d51517f30)
at /home/pq/git/weston/libweston/compositor.c:6925
#18 0x0000558d504f3c55 in main (argc=<optimized out>, argv=<optimized out>)
at /home/pq/git/weston/compositor/main.c:2752


#5 weston_debug_scope_timestamp (scope=0x558d51518770, buf=0x7ffe718ca4e0 "[2018-08-07 15:03:25.484][(\026\265\177",
len=128) at /home/pq/git/weston/libweston/weston-debug.c:717
717 snprintf(buf, len, "[%s.%03ld][%s]", string,
(gdb) print *scope
$1 = {
name = 0x80100016c <error: Cannot access memory at address 0x80100016c>,
desc = 0x980000001d <error: Cannot access memory at address 0x980000001d>,
begin_cb = 0x21006f0101,
user_data = 0x6572662f67726f2f,
stream_list = {
prev = 0x706f746b73656465,
next = 0x2f316e69676f6c2f
},
compositor_link = {
prev = 0x2f6e6f6973736573,
next = 0x32
}
}

That looks like it might be the scope already destroyed and dangling.
Post by Pekka Paalanen
Post by Daniel Stone
weston_compositor_destroy(wet.compositor);
Otherwise still good.
Thanks,
pq
Daniel Stone
2018-07-20 19:03:34 UTC
Permalink
Shift up our calculation of the flags we use for atomic commits. We will
later use this to differentiate between test-only and full commits when
printing debug information inside drm_output_state_apply_atomic.

Signed-off-by: Daniel Stone <***@collabora.com>
---
libweston/compositor-drm.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index e27671437..653d13e0c 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -2499,12 +2499,24 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,
struct drm_output_state *output_state, *tmp;
struct drm_plane *plane;
drmModeAtomicReq *req = drmModeAtomicAlloc();
- uint32_t flags = 0;
+ uint32_t flags;
int ret = 0;

if (!req)
return -1;

+ switch (mode) {
+ case DRM_STATE_APPLY_SYNC:
+ flags = 0;
+ break;
+ case DRM_STATE_APPLY_ASYNC:
+ flags = DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
+ break;
+ case DRM_STATE_TEST_ONLY:
+ flags = DRM_MODE_ATOMIC_TEST_ONLY;
+ break;
+ }
+
if (b->state_invalid) {
struct weston_head *head_base;
struct drm_head *head;
@@ -2597,17 +2609,6 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,
goto out;
}

- switch (mode) {
- case DRM_STATE_APPLY_SYNC:
- break;
- case DRM_STATE_APPLY_ASYNC:
- flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
- break;
- case DRM_STATE_TEST_ONLY:
- flags |= DRM_MODE_ATOMIC_TEST_ONLY;
- break;
- }
-
ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);

/* Test commits do not take ownership of the state; return
--
2.17.1
Pekka Paalanen
2018-08-07 10:34:28 UTC
Permalink
On Fri, 20 Jul 2018 20:03:34 +0100
Post by Daniel Stone
Shift up our calculation of the flags we use for atomic commits. We will
later use this to differentiate between test-only and full commits when
printing debug information inside drm_output_state_apply_atomic.
---
libweston/compositor-drm.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index e27671437..653d13e0c 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -2499,12 +2499,24 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,
struct drm_output_state *output_state, *tmp;
struct drm_plane *plane;
drmModeAtomicReq *req = drmModeAtomicAlloc();
- uint32_t flags = 0;
+ uint32_t flags;
int ret = 0;
if (!req)
return -1;
+ switch (mode) {
+ flags = 0;
+ break;
+ flags = DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
+ break;
+ flags = DRM_MODE_ATOMIC_TEST_ONLY;
+ break;
+ }
+
if (b->state_invalid) {
struct weston_head *head_base;
struct drm_head *head;
@@ -2597,17 +2609,6 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,
goto out;
}
- switch (mode) {
- break;
- flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
- break;
- flags |= DRM_MODE_ATOMIC_TEST_ONLY;
- break;
- }
-
ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
/* Test commits do not take ownership of the state; return
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>


Thanks,
pq
Daniel Stone
2018-07-20 19:03:35 UTC
Permalink
Add a 'drm-debug' scope which prints verbose information about the DRM
backend's repaint cycle, including the decision tree on how views are
assigned (or not) to planes.

Signed-off-by: Daniel Stone <***@collabora.com>
---
libweston/compositor-drm.c | 233 ++++++++++++++++++++++++++++++++-----
1 file changed, 206 insertions(+), 27 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 653d13e0c..2cadf036c 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -51,6 +51,7 @@

#include "compositor.h"
#include "compositor-drm.h"
+#include "weston-debug.h"
#include "shared/helpers.h"
#include "shared/timespec-util.h"
#include "gl-renderer.h"
@@ -73,6 +74,9 @@
#define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
#endif

+#define drm_debug(b, ...) \
+ weston_debug_scope_printf((b)->debug, __VA_ARGS__)
+
#define MAX_CLONED_CONNECTORS 4

/**
@@ -302,6 +306,8 @@ struct drm_backend {
bool shutting_down;

bool aspect_ratio_supported;
+
+ struct weston_debug_scope *debug;
};

struct drm_mode {
@@ -2358,6 +2364,9 @@ crtc_add_prop(drmModeAtomicReq *req, struct drm_output *output,

ret = drmModeAtomicAddProperty(req, output->crtc_id, info->prop_id,
val);
+ drm_debug(output->backend, "\t\t\t[CRTC:%d] %d (%s) -> %lld (0x%llx)\n",
+ output->crtc_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
return (ret <= 0) ? -1 : 0;
}

@@ -2373,6 +2382,9 @@ connector_add_prop(drmModeAtomicReq *req, struct drm_head *head,

ret = drmModeAtomicAddProperty(req, head->connector_id,
info->prop_id, val);
+ drm_debug(head->backend, "\t\t\t[CONN:%d] %d (%s) -> %lld (0x%llx)\n",
+ head->connector_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
return (ret <= 0) ? -1 : 0;
}

@@ -2388,6 +2400,9 @@ plane_add_prop(drmModeAtomicReq *req, struct drm_plane *plane,

ret = drmModeAtomicAddProperty(req, plane->plane_id, info->prop_id,
val);
+ drm_debug(plane->backend, "\t\t\t[PLANE:%d] %d (%s) -> %lld (0x%llx)\n",
+ plane->plane_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
return (ret <= 0) ? -1 : 0;
}

@@ -2406,6 +2421,9 @@ drm_mode_ensure_blob(struct drm_backend *backend, struct drm_mode *mode)
if (ret != 0)
weston_log("failed to create mode property blob: %m\n");

+ drm_debug(backend, "\t\t\t[atomic] created new mode blob %ld for %s",
+ (unsigned long) mode->blob_id, mode->mode_info.name);
+
return ret;
}

@@ -2415,17 +2433,23 @@ drm_output_apply_state_atomic(struct drm_output_state *state,
uint32_t *flags)
{
struct drm_output *output = state->output;
- struct drm_backend *backend = to_drm_backend(output->base.compositor);
+ struct drm_backend *b = to_drm_backend(output->base.compositor);
struct drm_plane_state *plane_state;
struct drm_mode *current_mode = to_drm_mode(output->base.current_mode);
struct drm_head *head;
int ret = 0;

- if (state->dpms != output->state_cur->dpms)
+ drm_debug(b, "\t\t[atomic] %s output %d (%s) state\n",
+ (*flags & DRM_MODE_ATOMIC_TEST_ONLY) ? "testing" : "applying",
+ output->base.id, output->base.name);
+
+ if (state->dpms != output->state_cur->dpms) {
+ drm_debug(b, "\t\t\t[atomic] DPMS state differs, modeset OK\n");
*flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
+ }

if (state->dpms == WESTON_DPMS_ON) {
- ret = drm_mode_ensure_blob(backend, current_mode);
+ ret = drm_mode_ensure_blob(b, current_mode);
if (ret != 0)
return ret;

@@ -2523,6 +2547,9 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,
uint32_t *unused;
int err;

+ drm_debug(b, "\t\t[atomic] previous state invalid; "
+ "starting with fresh state\n");
+
/* If we need to reset all our state (e.g. because we've
* just started, or just been VT-switched in), explicitly
* disable all the CRTCs and connectors we aren't using. */
@@ -2535,9 +2562,14 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,

head = to_drm_head(head_base);

+ drm_debug(b, "\t\t[atomic] disabling inactive head %s\n",
+ head_base->name);
+
info = &head->props_conn[WDRM_CONNECTOR_CRTC_ID];
err = drmModeAtomicAddProperty(req, head->connector_id,
info->prop_id, 0);
+ drm_debug(b, "\t\t\t[CONN:%d] %d (%s) -> 0\n",
+ head->connector_id, info->prop_id, info->name);
if (err <= 0)
ret = -1;
}
@@ -2574,12 +2606,19 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,
continue;
}

+ drm_debug(b, "\t\t[atomic] disabling unused CRTC %d\n",
+ *unused);
+
+ drm_debug(b, "\t\t\t[CRTC:%d] %d (%s) -> 0\n",
+ *unused, info->prop_id, info->name);
err = drmModeAtomicAddProperty(req, *unused,
info->prop_id, 0);
if (err <= 0)
ret = -1;

info = &infos[WDRM_CRTC_MODE_ID];
+ drm_debug(b, "\t\t\t[CRTC:%d] %d (%s) -> 0\n",
+ *unused, info->prop_id, info->name);
err = drmModeAtomicAddProperty(req, *unused,
info->prop_id, 0);
if (err <= 0)
@@ -2591,6 +2630,8 @@ drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,
/* Disable all the planes; planes which are being used will
* override this state in the output-state application. */
wl_list_for_each(plane, &b->plane_list, link) {
+ drm_debug(b, "\t\t[atomic] starting with plane %d disabled\n",
+ plane->plane_id);
plane_add_prop(req, plane, WDRM_PLANE_CRTC_ID, 0);
plane_add_prop(req, plane, WDRM_PLANE_FB_ID, 0);
}
@@ -2969,10 +3010,15 @@ drm_repaint_begin(struct weston_compositor *compositor)
{
struct drm_backend *b = to_drm_backend(compositor);
struct drm_pending_state *ret;
+ char *scene_graph = weston_compositor_print_scene_graph(compositor);

ret = drm_pending_state_alloc(b);
b->repaint_data = ret;

+ drm_debug(b, "[repaint] Beignning repaint; pending_state %p\n", ret);
+ drm_debug(b, "%s", scene_graph);
+ free(scene_graph);
+
return ret;
}

@@ -2992,6 +3038,7 @@ drm_repaint_flush(struct weston_compositor *compositor, void *repaint_data)
struct drm_pending_state *pending_state = repaint_data;

drm_pending_state_apply(pending_state);
+ drm_debug(b, "[repaint] flushed pending_state %p\n", pending_state);
b->repaint_data = NULL;
}

@@ -3008,6 +3055,7 @@ drm_repaint_cancel(struct weston_compositor *compositor, void *repaint_data)
struct drm_pending_state *pending_state = repaint_data;

drm_pending_state_free(pending_state);
+ drm_debug(b, "[repaint] cancel pending_state %p\n", pending_state);
b->repaint_data = NULL;
}

@@ -3051,12 +3099,21 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
struct drm_fb *fb;
unsigned int i;
int ret;
+ enum {
+ NO_PLANES,
+ NO_PLANES_WITH_FORMAT,
+ NO_PLANES_ACCEPTED,
+ PLACED_ON_PLANE,
+ } availability = NO_PLANES;

assert(!b->sprites_are_broken);

fb = drm_fb_get_from_view(output_state, ev);
- if (!fb)
+ if (!fb) {
+ drm_debug(b, "\t\t\t\t[overlay] not placing view %p on overlay: "
+ " couldn't get fb\n", ev);
return NULL;
+ }

wl_list_for_each(p, &b->plane_list, link) {
if (p->type != WDRM_PLANE_TYPE_OVERLAY)
@@ -3065,6 +3122,15 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
if (!drm_plane_is_available(p, output))
continue;

+ state = drm_output_state_get_plane(output_state, p);
+ if (state->fb) {
+ state = NULL;
+ continue;
+ }
+
+ if (availability == NO_PLANES)
+ availability = NO_PLANES_WITH_FORMAT;
+
/* Check whether the format is supported */
for (i = 0; i < p->count_formats; i++) {
unsigned int j;
@@ -3085,15 +3151,14 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
if (i == p->count_formats)
continue;

- state = drm_output_state_get_plane(output_state, p);
- if (state->fb) {
- state = NULL;
- continue;
- }
+ if (availability == NO_PLANES_WITH_FORMAT)
+ availability = NO_PLANES_ACCEPTED;

state->ev = ev;
state->output = output;
if (!drm_plane_state_coords_for_view(state, ev)) {
+ drm_debug(b, "\t\t\t\t[overlay] not placing view %p on overlay: "
+ "unsuitable transform\n", ev);
drm_plane_state_put_back(state);
state = NULL;
continue;
@@ -3101,6 +3166,8 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
if (!b->atomic_modeset &&
(state->src_w != state->dest_w << 16 ||
state->src_h != state->dest_h << 16)) {
+ drm_debug(b, "\t\t\t\t[overlay] not placing view %p on overlay: "
+ "no scaling without atomic\n", ev);
drm_plane_state_put_back(state);
state = NULL;
continue;
@@ -3114,17 +3181,48 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,

/* In planes-only mode, we don't have an incremental state to
* test against, so we just hope it'll work. */
- if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY)
+ if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
+ drm_debug(b, "\t\t\t\t[overlay] provisionally placing "
+ "view %p on overlay %d in planes-only mode\n",
+ ev, p->plane_id);
+ availability = PLACED_ON_PLANE;
goto out;
+ }

ret = drm_pending_state_test(output_state->pending_state);
- if (ret == 0)
+ if (ret == 0) {
+ drm_debug(b, "\t\t\t\t[overlay] provisionally placing "
+ "view %p on overlay %d in mixed mode\n",
+ ev, p->plane_id);
+ availability = PLACED_ON_PLANE;
goto out;
+ }
+
+ drm_debug(b, "\t\t\t\t[overlay] not placing view %p on overlay %d "
+ "in mixed mode: kernel test failed\n",
+ ev, p->plane_id);

drm_plane_state_put_back(state);
state = NULL;
}

+ switch (availability) {
+ case NO_PLANES:
+ drm_debug(b, "\t\t\t\t[overlay] not placing view %p on overlay: "
+ "no free overlay planes\n", ev);
+ break;
+ case NO_PLANES_WITH_FORMAT:
+ drm_debug(b, "\t\t\t\t[overlay] not placing view %p on overlay: "
+ "no free overlay planes matching format 0x%lx, "
+ "modifier 0x%llx\n",
+ ev, (unsigned long) fb->format,
+ (unsigned long long) fb->modifier);
+ break;
+ case NO_PLANES_ACCEPTED:
+ case PLACED_ON_PLANE:
+ break;
+ }
+
out:
drm_fb_unref(fb);
return state;
@@ -3193,13 +3291,23 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state,
if (b->gbm == NULL)
return NULL;

- if (ev->surface->buffer_ref.buffer == NULL)
+ if (ev->surface->buffer_ref.buffer == NULL) {
+ drm_debug(b, "\t\t\t\t[cursor] not assigning view %p to cursor plane "
+ "(no buffer available)\n", ev);
return NULL;
+ }
shmbuf = wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource);
- if (!shmbuf)
+ if (!shmbuf) {
+ drm_debug(b, "\t\t\t\t[cursor] not assigning view %p to cursor plane "
+ "(buffer isn't SHM)\n", ev);
return NULL;
- if (wl_shm_buffer_get_format(shmbuf) != WL_SHM_FORMAT_ARGB8888)
+ }
+ if (wl_shm_buffer_get_format(shmbuf) != WL_SHM_FORMAT_ARGB8888) {
+ drm_debug(b, "\t\t\t\t[cursor] not assigning view %p to cursor plane "
+ "(format 0x%lx unsuitable)\n",
+ ev, (unsigned long) wl_shm_buffer_get_format(shmbuf));
return NULL;
+ }

plane_state =
drm_output_state_get_plane(output_state, output->cursor_plane);
@@ -3217,8 +3325,11 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state,
plane_state->src_w > (unsigned) b->cursor_width << 16 ||
plane_state->src_h > (unsigned) b->cursor_height << 16 ||
plane_state->src_w != plane_state->dest_w << 16 ||
- plane_state->src_h != plane_state->dest_h << 16)
+ plane_state->src_h != plane_state->dest_h << 16) {
+ drm_debug(b, "\t\t\t\t[cursor] not assigning view %p to cursor plane "
+ "(positioning requires cropping or scaling)\n", ev);
goto err;
+ }

/* Since we're setting plane state up front, we need to work out
* whether or not we need to upload a new cursor. We can't use the
@@ -3241,8 +3352,10 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state,
plane_state->fb =
drm_fb_ref(output->gbm_cursor_fb[output->current_cursor]);

- if (needs_update)
+ if (needs_update) {
+ drm_debug(b, "\t\t\t\t[cursor] copying new content to cursor BO\n");
cursor_bo_update(plane_state, ev);
+ }

/* The cursor API is somewhat special: in cursor_bo_update(), we upload
* a buffer which is always cursor_width x cursor_height, even if the
@@ -3253,6 +3366,9 @@ drm_output_prepare_cursor_view(struct drm_output_state *output_state,
plane_state->dest_w = b->cursor_width;
plane_state->dest_h = b->cursor_height;

+ drm_debug(b, "\t\t\t\t[cursor] provisionally assigned view %p to cursor\n",
+ ev);
+
return plane_state;

err:
@@ -3348,18 +3464,29 @@ drm_output_propose_state(struct weston_output *output_base,
if (!scanout_fb ||
(scanout_fb->type != BUFFER_GBM_SURFACE &&
scanout_fb->type != BUFFER_PIXMAN_DUMB)) {
+ drm_debug(b, "\t\t[state] cannot propose mixed mode: "
+ "for output %s (%d): no previous renderer "
+ "fb\n",
+ output->base.name, output->base.id);
drm_output_state_free(state);
return NULL;
}

if (scanout_fb->width != output_base->current_mode->width ||
scanout_fb->height != output_base->current_mode->height) {
+ drm_debug(b, "\t\t[state] cannot propose mixed mode "
+ "for output %s (%d): previous fb has "
+ "different size\n",
+ output->base.name, output->base.id);
drm_output_state_free(state);
return NULL;
}

scanout_state = drm_plane_state_duplicate(state,
plane->state_cur);
+ drm_debug(b, "\t\t[state] using renderer FB ID %d for mixed "
+ "mode for output %s (%d)\n",
+ scanout_fb->fb_id, output->base.name, output->base.id);
}

/*
@@ -3385,18 +3512,31 @@ drm_output_propose_state(struct weston_output *output_base,
bool totally_occluded = false;
bool overlay_occluded = false;

+ drm_debug(b, "\t\t\t[view] evaluating view %p for "
+ "output %s (%d)\n",
+ ev, output->base.name, output->base.id);
+
/* If this view doesn't touch our output at all, there's no
* reason to do anything with it. */
- if (!(ev->output_mask & (1u << output->base.id)))
+ if (!(ev->output_mask & (1u << output->base.id))) {
+ drm_debug(b, "\t\t\t\t[view] ignoring view %p "
+ "(not on our output)\n", ev);
continue;
+ }

/* We only assign planes to views which are exclusively present
* on our output. */
- if (ev->output_mask != (1u << output->base.id))
+ if (ev->output_mask != (1u << output->base.id)) {
+ drm_debug(b, "\t\t\t\t[view] not assigning view %p to plane "
+ "(on multiple outputs)\n", ev);
force_renderer = true;
+ }

- if (!ev->surface->buffer_ref.buffer)
+ if (!ev->surface->buffer_ref.buffer) {
+ drm_debug(b, "\t\t\t\t[view] not assigning view %p to plane "
+ "(no buffer available)\n", ev);
force_renderer = true;
+ }

/* Ignore views we know to be totally occluded. */
pixman_region32_init(&clipped_view);
@@ -3409,6 +3549,8 @@ drm_output_propose_state(struct weston_output *output_base,
&occluded_region);
totally_occluded = !pixman_region32_not_empty(&surface_overlap);
if (totally_occluded) {
+ drm_debug(b, "\t\t\t\t[view] ignoring view %p "
+ "(occluded on our output)\n", ev);
pixman_region32_fini(&surface_overlap);
pixman_region32_fini(&clipped_view);
continue;
@@ -3419,8 +3561,11 @@ drm_output_propose_state(struct weston_output *output_base,
* be part of, or occluded by, it, and cannot go on a plane. */
pixman_region32_intersect(&surface_overlap, &renderer_region,
&clipped_view);
- if (pixman_region32_not_empty(&surface_overlap))
+ if (pixman_region32_not_empty(&surface_overlap)) {
+ drm_debug(b, "\t\t\t\t[view] not assigning view %p to plane "
+ "(occluded by renderer views)\n", ev);
force_renderer = true;
+ }

/* We do not control the stacking order of overlay planes;
* the scanout plane is strictly stacked bottom and the cursor
@@ -3429,8 +3574,11 @@ drm_output_propose_state(struct weston_output *output_base,
* planes overlapping each other. */
pixman_region32_intersect(&surface_overlap, &occluded_region,
&clipped_view);
- if (pixman_region32_not_empty(&surface_overlap))
+ if (pixman_region32_not_empty(&surface_overlap)) {
+ drm_debug(b, "\t\t\t\t[view] not assigning view %p to plane "
+ "(occluded by other overlay planes)\n", ev);
overlay_occluded = true;
+ }
pixman_region32_fini(&surface_overlap);

/* The cursor plane is 'special' in the sense that we can still
@@ -3442,10 +3590,16 @@ drm_output_propose_state(struct weston_output *output_base,
/* If sprites are disabled or the view is not fully opaque, we
* must put the view into the renderer - unless it has already
* been placed in the cursor plane, which can handle alpha. */
- if (!ps && !planes_ok)
+ if (!ps && !planes_ok) {
+ drm_debug(b, "\t\t\t\t[view] not assigning view %p to plane "
+ "(precluded by mode)\n", ev);
force_renderer = true;
- if (!ps && !drm_view_is_opaque(ev))
+ }
+ if (!ps && !drm_view_is_opaque(ev)) {
+ drm_debug(b, "\t\t\t\t[view] not assigning view %p to plane "
+ "(view not fully opaque)\n", ev);
force_renderer = true;
+ }

/* Only try to place scanout surfaces in planes-only mode; in
* mixed mode, we have already failed to place a view on the
@@ -3478,6 +3632,9 @@ drm_output_propose_state(struct weston_output *output_base,
* check if this is OK, and add ourselves to the renderer
* region if so. */
if (!renderer_ok) {
+ drm_debug(b, "\t\t[view] failing state generation: "
+ "placing view %p to renderer not allowed\n",
+ ev);
pixman_region32_fini(&clipped_view);
goto err_region;
}
@@ -3497,8 +3654,11 @@ drm_output_propose_state(struct weston_output *output_base,

/* Check to see if this state will actually work. */
ret = drm_pending_state_test(state->pending_state);
- if (ret != 0)
+ if (ret != 0) {
+ drm_debug(b, "\t\t[view] failing state generation: "
+ "atomic test not OK\n");
goto err;
+ }

/* Counterpart to duplicating scanout state at the top of this
* function: if we have taken a renderer framebuffer and placed it in
@@ -3531,12 +3691,20 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data)
struct weston_view *ev;
struct weston_plane *primary = &output_base->compositor->primary_plane;

+ drm_debug(b, "\t[repaint] preparing state for output %s (%d)\n",
+ output_base->name, output_base->id);
+
if (!b->sprites_are_broken) {
state = drm_output_propose_state(output_base, pending_state,
DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
- if (!state)
+ if (!state) {
+ drm_debug(b, "\t[repaint] could not build planes-only "
+ "state, trying mixed\n");
state = drm_output_propose_state(output_base, pending_state,
DRM_OUTPUT_PROPOSE_STATE_MIXED);
+ }
+ } else {
+ drm_debug(b, "\t[state] no overlay plane support\n");
}

if (!state)
@@ -3583,10 +3751,16 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data)
}
}

- if (target_plane)
+ if (target_plane) {
+ drm_debug(b, "\t[repaint] view %p on %s plane %d\n",
+ ev, plane_type_enums[target_plane->type].name,
+ target_plane->plane_id);
weston_view_move_to_plane(ev, &target_plane->base);
- else
+ } else {
+ drm_debug(b, "\t[repaint] view %p using renderer "
+ "composition\n", ev);
weston_view_move_to_plane(ev, primary);
+ }

if (!target_plane ||
target_plane->type == WDRM_PLANE_TYPE_CURSOR) {
@@ -6262,6 +6436,7 @@ drm_destroy(struct weston_compositor *ec)

destroy_sprites(b);

+ weston_debug_scope_destroy(b->debug);
weston_compositor_shutdown(ec);

wl_list_for_each_safe(base, next, &ec->head_list, compositor_link)
@@ -6723,6 +6898,10 @@ drm_backend_create(struct weston_compositor *compositor,
b->pageflip_timeout = config->pageflip_timeout;
b->use_pixman_shadow = config->use_pixman_shadow;

+ b->debug = weston_compositor_add_debug_scope(compositor, "drm-backend",
+ "Debug messages from DRM/KMS backend\n",
+ NULL, NULL);
+
compositor->backend = &b->base;

if (parse_gbm_format(config->gbm_format, GBM_FORMAT_XRGB8888, &b->gbm_format) < 0)
--
2.17.1
Pekka Paalanen
2018-08-07 12:09:38 UTC
Permalink
On Fri, 20 Jul 2018 20:03:35 +0100
Post by Daniel Stone
Add a 'drm-debug' scope which prints verbose information about the DRM
backend's repaint cycle, including the decision tree on how views are
assigned (or not) to planes.
---
libweston/compositor-drm.c | 233 ++++++++++++++++++++++++++++++++-----
1 file changed, 206 insertions(+), 27 deletions(-)
Hi,

lots of output from this one, nice!
Post by Daniel Stone
diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 653d13e0c..2cadf036c 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -51,6 +51,7 @@
#include "compositor.h"
#include "compositor-drm.h"
+#include "weston-debug.h"
#include "shared/helpers.h"
#include "shared/timespec-util.h"
#include "gl-renderer.h"
@@ -73,6 +74,9 @@
#define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
#endif
+#define drm_debug(b, ...) \
+ weston_debug_scope_printf((b)->debug, __VA_ARGS__)
+
#define MAX_CLONED_CONNECTORS 4
/**
@@ -302,6 +306,8 @@ struct drm_backend {
bool shutting_down;
bool aspect_ratio_supported;
+
+ struct weston_debug_scope *debug;
};
struct drm_mode {
@@ -2358,6 +2364,9 @@ crtc_add_prop(drmModeAtomicReq *req, struct drm_output *output,
ret = drmModeAtomicAddProperty(req, output->crtc_id, info->prop_id,
val);
+ drm_debug(output->backend, "\t\t\t[CRTC:%d] %d (%s) -> %lld (0x%llx)\n",
+ output->crtc_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
This is using %lld to print (unsigned long long), should it not be %llu
instead?

All the property ids are unsigned as well, so %d -> %u?
Post by Daniel Stone
return (ret <= 0) ? -1 : 0;
}
@@ -2373,6 +2382,9 @@ connector_add_prop(drmModeAtomicReq *req, struct drm_head *head,
ret = drmModeAtomicAddProperty(req, head->connector_id,
info->prop_id, val);
+ drm_debug(head->backend, "\t\t\t[CONN:%d] %d (%s) -> %lld (0x%llx)\n",
+ head->connector_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
%d -> %u
%lld -> %llu
Post by Daniel Stone
return (ret <= 0) ? -1 : 0;
}
@@ -2388,6 +2400,9 @@ plane_add_prop(drmModeAtomicReq *req, struct drm_plane *plane,
ret = drmModeAtomicAddProperty(req, plane->plane_id, info->prop_id,
val);
+ drm_debug(plane->backend, "\t\t\t[PLANE:%d] %d (%s) -> %lld (0x%llx)\n",
+ plane->plane_id, info->prop_id, info->name,
+ (unsigned long long) val, (unsigned long long) val);
%d -> %u
%lld -> %llu
Post by Daniel Stone
return (ret <= 0) ? -1 : 0;
}
@@ -2406,6 +2421,9 @@ drm_mode_ensure_blob(struct drm_backend *backend, struct drm_mode *mode)
if (ret != 0)
weston_log("failed to create mode property blob: %m\n");
+ drm_debug(backend, "\t\t\t[atomic] created new mode blob %ld for %s",
+ (unsigned long) mode->blob_id, mode->mode_info.name);
%ld -> %u without the cast?

There are more of these kinds below, should I point them out?
Post by Daniel Stone
+
return ret;
}
...
Post by Daniel Stone
@@ -2969,10 +3010,15 @@ drm_repaint_begin(struct weston_compositor *compositor)
{
struct drm_backend *b = to_drm_backend(compositor);
struct drm_pending_state *ret;
+ char *scene_graph = weston_compositor_print_scene_graph(compositor);
Maybe you'd want to make the call to
weston_compositor_print_scene_graph() conditional on the debug scope
being enabled? To not incur the work while not being debugged.
Post by Daniel Stone
ret = drm_pending_state_alloc(b);
b->repaint_data = ret;
+ drm_debug(b, "[repaint] Beignning repaint; pending_state %p\n", ret);
Typoed "Beginning".
Post by Daniel Stone
+ drm_debug(b, "%s", scene_graph);
+ free(scene_graph);
+
return ret;
}
...
Post by Daniel Stone
@@ -3531,12 +3691,20 @@ drm_assign_planes(struct weston_output *output_base, void *repaint_data)
struct weston_view *ev;
struct weston_plane *primary = &output_base->compositor->primary_plane;
+ drm_debug(b, "\t[repaint] preparing state for output %s (%d)\n",
+ output_base->name, output_base->id);
+
if (!b->sprites_are_broken) {
state = drm_output_propose_state(output_base, pending_state,
DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
- if (!state)
+ if (!state) {
+ drm_debug(b, "\t[repaint] could not build planes-only "
+ "state, trying mixed\n");
state = drm_output_propose_state(output_base, pending_state,
DRM_OUTPUT_PROPOSE_STATE_MIXED);
Should there be a note if mixed mode failed?
Post by Daniel Stone
+ }
+ } else {
+ drm_debug(b, "\t[state] no overlay plane support\n");
}
if (!state)
Looks good aside from the minor details.


Thanks,
pq
Daniel Stone
2018-07-20 19:03:25 UTC
Permalink
From: Pekka Paalanen <***@iki.fi>

Let users enable the compositor debug protocol on the compositor command
line. This allows weston-debug tool to work.

Signed-off-by: Pekka Paalanen <***@iki.fi>
Signed-off-by: Maniraj Devadoss <***@in.bosch.com>
Reviewed-by: Pekka Paalanen <***@collabora.co.uk>
---
compositor/main.c | 7 +++++++
man/weston.man | 11 +++++++++++
2 files changed, 18 insertions(+)

diff --git a/compositor/main.c b/compositor/main.c
index b5b4fc594..2f34e1115 100644
--- a/compositor/main.c
+++ b/compositor/main.c
@@ -60,6 +60,7 @@
#include "compositor-x11.h"
#include "compositor-wayland.h"
#include "windowed-output-api.h"
+#include "weston-debug.h"

#define WINDOW_TITLE "Weston Compositor"

@@ -508,6 +509,7 @@ usage(int error_code)
" -c, --config=FILE\tConfig file to load, defaults to weston.ini\n"
" --no-config\t\tDo not read weston.ini\n"
" --wait-for-debugger\tRaise SIGSTOP on start-up\n"
+ " --debug\t\tEnable debug extension\n"
" -h, --help\t\tThis help message\n\n");

#if defined(BUILD_DRM_COMPOSITOR)
@@ -2375,6 +2377,7 @@ int main(int argc, char *argv[])
char *socket_name = NULL;
int32_t version = 0;
int32_t noconfig = 0;
+ int32_t debug_protocol = 0;
int32_t numlock_on;
char *config_file = NULL;
struct weston_config *config = NULL;
@@ -2399,6 +2402,7 @@ int main(int argc, char *argv[])
{ WESTON_OPTION_BOOLEAN, "no-config", 0, &noconfig },
{ WESTON_OPTION_STRING, "config", 'c', &config_file },
{ WESTON_OPTION_BOOLEAN, "wait-for-debugger", 0, &wait_for_debugger },
+ { WESTON_OPTION_BOOLEAN, "debug", 0, &debug_protocol },
};

wl_list_init(&wet.layoutput_list);
@@ -2486,6 +2490,9 @@ int main(int argc, char *argv[])
}
segv_compositor = wet.compositor;

+ if (debug_protocol)
+ weston_compositor_enable_debug_protocol(wet.compositor);
+
if (weston_compositor_init_config(wet.compositor, config) < 0)
goto out;

diff --git a/man/weston.man b/man/weston.man
index 596041dff..8a0e4a6f8 100644
--- a/man/weston.man
+++ b/man/weston.man
@@ -133,6 +133,17 @@ If also
.B --no-config
is given, no configuration file will be read.
.TP
+.BR \-\-debug
+Enable debug protocol extension
+.I weston_debug_v1
+which any client can use to receive debugging messages from the compositor.
+
+.B WARNING:
+This is risky for two reasons. First, a client may cause a denial-of-service
+blocking the compositor by providing an unsuitable file descriptor, and
+second, the debug messages may expose sensitive information. This option
+should not be used in production.
+.TP
.BR \-\-version
Print the program version.
.TP
--
2.17.1
Daniel Stone
2018-07-20 19:03:32 UTC
Permalink
As a counterpart to weston_layer_set_mask_infinite(), returning if the
mask is the same as what is set.

Signed-off-by: Daniel Stone <***@collabora.com>
---
libweston/compositor.c | 9 +++++++++
libweston/compositor.h | 3 +++
2 files changed, 12 insertions(+)

diff --git a/libweston/compositor.c b/libweston/compositor.c
index 8768f67a0..0c147d4a6 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -2746,6 +2746,15 @@ weston_layer_set_mask_infinite(struct weston_layer *layer)
UINT32_MAX, UINT32_MAX);
}

+WL_EXPORT bool
+weston_layer_mask_is_infinite(struct weston_layer *layer)
+{
+ return layer->mask.x1 == INT32_MIN &&
+ layer->mask.y1 == INT32_MIN &&
+ layer->mask.x2 == INT32_MIN + UINT32_MAX &&
+ layer->mask.y2 == INT32_MIN + UINT32_MAX;
+}
+
WL_EXPORT void
weston_output_schedule_repaint(struct weston_output *output)
{
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 83448b70f..67a835713 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1686,6 +1686,9 @@ weston_layer_set_mask(struct weston_layer *layer, int x, int y, int width, int h
void
weston_layer_set_mask_infinite(struct weston_layer *layer);

+bool
+weston_layer_mask_is_infinite(struct weston_layer *layer);
+
void
weston_plane_init(struct weston_plane *plane,
struct weston_compositor *ec,
--
2.17.1
Pekka Paalanen
2018-08-06 14:27:10 UTC
Permalink
On Fri, 20 Jul 2018 20:03:32 +0100
Post by Daniel Stone
As a counterpart to weston_layer_set_mask_infinite(), returning if the
mask is the same as what is set.
---
libweston/compositor.c | 9 +++++++++
libweston/compositor.h | 3 +++
2 files changed, 12 insertions(+)
diff --git a/libweston/compositor.c b/libweston/compositor.c
index 8768f67a0..0c147d4a6 100644
--- a/libweston/compositor.c
+++ b/libweston/compositor.c
@@ -2746,6 +2746,15 @@ weston_layer_set_mask_infinite(struct weston_layer *layer)
UINT32_MAX, UINT32_MAX);
}
+WL_EXPORT bool
+weston_layer_mask_is_infinite(struct weston_layer *layer)
+{
+ return layer->mask.x1 == INT32_MIN &&
+ layer->mask.y1 == INT32_MIN &&
+ layer->mask.x2 == INT32_MIN + UINT32_MAX &&
+ layer->mask.y2 == INT32_MIN + UINT32_MAX;
+}
Hi Daniel,

I suppose this repeats what weston_layer_set_mask_infinite() does, so

Reviewed-by: Pekka Paalanen <***@collabora.co.uk>

They are both overflowing int or maybe this one is implicitly
converting a negative to unsigned, but I believe it works in practice.
Wonder if Clang would disagree.


Thanks,
pq
Post by Daniel Stone
+
WL_EXPORT void
weston_output_schedule_repaint(struct weston_output *output)
{
diff --git a/libweston/compositor.h b/libweston/compositor.h
index 83448b70f..67a835713 100644
--- a/libweston/compositor.h
+++ b/libweston/compositor.h
@@ -1686,6 +1686,9 @@ weston_layer_set_mask(struct weston_layer *layer, int x, int y, int width, int h
void
weston_layer_set_mask_infinite(struct weston_layer *layer);
+bool
+weston_layer_mask_is_infinite(struct weston_layer *layer);
+
void
weston_plane_init(struct weston_plane *plane,
struct weston_compositor *ec,
Loading...