On Fri, 20 Jul 2018 20:03:33 +0100 Daniel Stone <[email protected]> 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 <[email protected]> > --- > 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
pgp1pT4cQLZK8.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
