[RFC] Weston sprite support

Kristian Høgsberg krh at bitplanet.net
Mon Jan 9 12:56:10 PST 2012


2012/1/9 Jesse Barnes <jbarnes at virtuousgeek.org>:
> On Fri, 6 Jan 2012 16:22:33 -0500
> Kristian Høgsberg <krh at bitplanet.net> wrote:
>> I'm concerned about make all of weston_output_repaint an output hook.
>> There's a lot of non-trivial (at least it took me a while to get it
>> right) region code in there to analyze the overlap of the surfaces to
>> minimize the overdraw and to enable the incremental repaint.  I think
>> we can just do a different type of hook though.  Maybe after
>> calculating the per-surface damage, we can call into the backend,
>> which can then look through the stack, decide whether and which
>> surfaces should use sprites, set that up and then clear their damage.
>> When the hook returns to weston_output_repaint, the repaint will then
>> skip those surfaces.
>>
>> For that to work I think we need two changes: the wl_list_for_each
>> that applies the per-surface damage (line 820) needs to move up above
>> setup_scanout_surface().  In fact, I think we can roll
>> setup_scanout_surface() into that hook and handle pageflipping to
>> client buffers in the same hook.  It's not all that different.
>> Attached a patch/sketch.
>
> So something like this is more like what you had in mind?

Yup, that looks promising.  One more thought:

The way we currently reference the scanout buffer needs to be done for
buffers we use for sprites as well.  Once a client attaches a buffer,
updates to the buffer contents may not be atomic, so clients should
wait for the buffer.release event before drawing to it again (if they
care).  Normally a buffer is released immediately when a new buffer is
attached, but in case of scanout buffers (framebuffers and sprite
buffers), the hw may still be scanning out from it, and we send the
release only when the pageflip event for the new buffer comes back to
the compositor.  The mechanism is only used by drm, so I think we
should pull it back into compositor-drm.c and then generalize it in
struct drm_buffer_reference.  Look for anything releated to
output->scanout_buffer*.  We can then use that to track the current
and next buffers and send out the buffer.release event, for both the
fb (as we do now) and the sprite buffers.

Kristian

> --
> Jesse Barnes, Intel Open Source Technology Center
>
> From 43134f5c35052e77369d13ee9a1179da26d3ca5f Mon Sep 17 00:00:00 2001
> From: Jesse Barnes <jbarnes at virtuousgeek.org>
> Date: Fri, 6 Jan 2012 11:24:20 -0800
> Subject: [PATCH] compositor: make surface setup a special case of sprite assignment
>
> Remove the surface prepare hook and add a new assign overlays hook.
> This lets back ends do more sophisticated surface management.
> ---
>  src/compositor-drm.c     |   46 ++++++++++++++++++++++++++++++++++++++--
>  src/compositor-wayland.c |    9 --------
>  src/compositor-x11.c     |    9 --------
>  src/compositor.c         |   52 ++++++++++++++++++++-------------------------
>  src/compositor.h         |    6 ++--
>  5 files changed, 69 insertions(+), 53 deletions(-)
>
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 433c644..61108fe 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -78,6 +78,19 @@ struct drm_output {
>        uint32_t pending_fs_surf_fb_id;
>  };
>
> +/*
> + * An output has a primary display plane plus zero or more sprites for
> + * blending display contents.
> + */
> +struct drm_output_sprite {
> +       uint32_t fb_id;
> +
> +       int32_t src_x, src_y;
> +       uint32_t src_w, src_h;
> +       uint32_t dest_x, dest_y;
> +       uint32_t dest_w, dest_h;
> +};
> +
>  static int
>  drm_output_prepare_render(struct weston_output *output_base)
>  {
> @@ -188,6 +201,35 @@ drm_output_prepare_scanout_surface(struct weston_output *output_base,
>  }
>
>  static int
> +drm_assign_overlays(struct weston_output *output)
> +{
> +       struct weston_compositor *ec = output->compositor;
> +       struct weston_surface *es;
> +
> +       es = container_of(ec->surface_list.next, struct weston_surface, link);
> +
> +       if (es->visual == WESTON_RGB_VISUAL) {
> +               if (!drm_output_prepare_scanout_surface(output, es))
> +                       weston_queue_scanout_surface(output, es);
> +       }
> +
> +       /*
> +        * Find a surface for each sprite in the output using some heuristics:
> +        * 1) size
> +        * 2) frequency of update
> +        * 3) opacity (though some hw might support alpha blending)
> +        * 4) clipping (this can be fixed with color keys)
> +        *
> +        * The idea is to save on blitting since this should save power.
> +        * If we can get a large video surface on the sprite for example,
> +        * the main display surface may not need to update at all, and
> +        * the client buffer can be used directly for the sprite surface
> +        * as we do for flipping full screen surfaces.
> +        */
> +       return 0;
> +}
> +
> +static int
>  drm_output_set_cursor(struct weston_output *output_base,
>                      struct weston_input_device *eid)
>  {
> @@ -548,11 +590,9 @@ create_output_for_connector(struct drm_compositor *ec,
>        wl_list_insert(ec->base.output_list.prev, &output->base.link);
>
>        output->pending_fs_surf_fb_id = 0;
> -       output->base.repaint = weston_output_repaint;
> +       output->base.assign_overlays = drm_assign_overlays;
>        output->base.prepare_render = drm_output_prepare_render;
>        output->base.present = drm_output_present;
> -       output->base.prepare_scanout_surface =
> -               drm_output_prepare_scanout_surface;
>        output->base.set_hardware_cursor = drm_output_set_cursor;
>        output->base.destroy = drm_output_destroy;
>
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index 4966017..a80670d 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -207,13 +207,6 @@ wayland_output_present(struct weston_output *output_base)
>  }
>
>  static int
> -wayland_output_prepare_scanout_surface(struct weston_output *output_base,
> -                                      struct weston_surface *es)
> -{
> -       return -1;
> -}
> -
> -static int
>  wayland_output_set_cursor(struct weston_output *output_base,
>                          struct weston_input_device *input)
>  {
> @@ -293,8 +286,6 @@ wayland_compositor_create_output(struct wayland_compositor *c,
>        output->base.repaint = weston_output_repaint;
>        output->base.prepare_render = wayland_output_prepare_render;
>        output->base.present = wayland_output_present;
> -       output->base.prepare_scanout_surface =
> -               wayland_output_prepare_scanout_surface;
>        output->base.set_hardware_cursor = wayland_output_set_cursor;
>        output->base.destroy = wayland_output_destroy;
>
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 1b23e83..d4850c8 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -231,13 +231,6 @@ x11_output_present(struct weston_output *output_base)
>  }
>
>  static int
> -x11_output_prepare_scanout_surface(struct weston_output *output_base,
> -                                  struct weston_surface *es)
> -{
> -       return -1;
> -}
> -
> -static int
>  x11_output_set_cursor(struct weston_output *output_base,
>                      struct weston_input_device *input)
>  {
> @@ -464,8 +457,6 @@ x11_compositor_create_output(struct x11_compositor *c, int x, int y,
>        output->base.repaint = weston_output_repaint;
>        output->base.prepare_render = x11_output_prepare_render;
>        output->base.present = x11_output_present;
> -       output->base.prepare_scanout_surface =
> -               x11_output_prepare_scanout_surface;
>        output->base.set_hardware_cursor = x11_output_set_cursor;
>        output->base.destroy = x11_output_destroy;
>
> diff --git a/src/compositor.c b/src/compositor.c
> index 7c41123..8a12748 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -733,13 +733,10 @@ out:
>        pixman_region32_fini(&cursor_region);
>  }
>
> -static int
> -setup_scanout_surface(struct weston_output *output, struct weston_surface *es)
> +WL_EXPORT int
> +weston_queue_scanout_surface(struct weston_output *output,
> +                            struct weston_surface *es)
>  {
> -       if (es->visual != WESTON_RGB_VISUAL ||
> -           output->prepare_scanout_surface(output, es) != 0)
> -               return -1;
> -
>        /* assert output->pending_scanout_buffer == NULL */
>        output->pending_scanout_buffer = es->buffer;
>        output->pending_scanout_buffer->busy_count++;
> @@ -789,30 +786,27 @@ weston_output_repaint(struct weston_output *output)
>
>        es = container_of(ec->surface_list.next, struct weston_surface, link);
>
> -       if (setup_scanout_surface(output, es) == 0)
> -               /* We're drawing nothing, just let the damage accumulate */
> -               return;
> -
> -       if (es->fullscreen_output == output) {
> -               if (es->width < output->current->width ||
> -                   es->height < output->current->height)
> -                       glClear(GL_COLOR_BUFFER_BIT);
> -               weston_surface_draw(es, output, &total_damage);
> -       } else {
> -               wl_list_for_each(es, &ec->surface_list, link) {
> -                       pixman_region32_copy(&es->damage, &total_damage);
> -                       pixman_region32_subtract(&total_damage, &total_damage, &es->opaque);
> -               }
> +       wl_list_for_each(es, &ec->surface_list, link) {
> +               pixman_region32_copy(&es->damage, &total_damage);
> +               pixman_region32_subtract(&total_damage, &total_damage,
> +                                        &es->opaque);
> +       }
>
> -               wl_list_for_each_reverse(es, &ec->surface_list, link) {
> -                       pixman_region32_init(&repaint);
> -                       pixman_region32_intersect(&repaint, &output->region,
> -                                                 &es->damage);
> -                       weston_surface_draw(es, output, &repaint);
> -                       pixman_region32_subtract(&es->damage,
> -                                                &es->damage, &output->region);
> -                       pixman_region32_fini(&repaint);
> -               }
> +       if (output->assign_overlays)
> +               /* This will queue flips for the fbs and sprites where
> +                * applicable and clear the damage for those surfaces.
> +                * The repaint loop below will repaint everything
> +                * else. */
> +               output->assign_overlays(output);
> +
> +       wl_list_for_each_reverse(es, &ec->surface_list, link) {
> +               pixman_region32_init(&repaint);
> +               pixman_region32_intersect(&repaint, &output->region,
> +                                         &es->damage);
> +               weston_surface_draw(es, output, &repaint);
> +               pixman_region32_subtract(&es->damage,
> +                                        &es->damage, &output->region);
> +               pixman_region32_fini(&repaint);
>        }
>
>        if (ec->fade.spring.current > 0.001)
> diff --git a/src/compositor.h b/src/compositor.h
> index 5608899..d8cd375 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -90,8 +90,7 @@ struct weston_output {
>        void (*repaint)(struct weston_output *output);
>        int (*prepare_render)(struct weston_output *output);
>        int (*present)(struct weston_output *output);
> -       int (*prepare_scanout_surface)(struct weston_output *output,
> -                                      struct weston_surface *es);
> +       int (*assign_overlays)(struct weston_output *output);
>        int (*set_hardware_cursor)(struct weston_output *output,
>                                   struct weston_input_device *input);
>        void (*destroy)(struct weston_output *output);
> @@ -492,5 +491,6 @@ typedef     void (*weston_zoom_done_func_t)(struct weston_zoom *zoom, void *data);
>  struct weston_zoom *
>  weston_zoom_run(struct weston_surface *surface, GLfloat start, GLfloat stop,
>                weston_zoom_done_func_t done, void *data);
> -
> +int weston_queue_scanout_surface(struct weston_output *output,
> +                                struct weston_surface *es);
>  #endif
> --
> 1.7.4.1
>
>


More information about the wayland-devel mailing list