[PATCH weston v5 12/14] compositor: Add scene-graph debug scope

[Pekka Paalanen]

On Fri, 20 Jul 2018 20:03:33 +0100
Daniel Stone <daniels at collabora.com> wrote:

> 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 <daniels at collabora.com>
> ---
>  libweston/compositor.c | 219 +++++++++++++++++++++++++++++++++++++++++
>  libweston/compositor.h |   4 +
>  2 files changed, 223 insertions(+)

Hi Daniel,

this looks nice, just a few minor issues below.


> 
> 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");

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.

> +		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.

> +
> +	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?

> +	} 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.

> +				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.

> +		}
> +
> +		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",

Three decimals would be enough.

The rest is good.

> +			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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180806/0bee766b/attachment.sig>