[Spice-devel] [PATCH 03/22] display: move more logic in display_channel_get_drawable()

Fabiano Fidêncio fidencio at redhat.com
Wed Dec 2 09:17:26 PST 2015


On Wed, Dec 2, 2015 at 5:19 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau at gmail.com>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/display-channel.c | 185 +++++++++++++++++++++++++++++++++++++++++++++++
>  server/display-channel.h |  15 ----
>  server/red_worker.c      | 172 -------------------------------------------
>  3 files changed, 185 insertions(+), 187 deletions(-)
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 2eeb021..43f2e0e 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -916,6 +916,152 @@ void display_channel_print_stats(DisplayChannel *display)
>  #endif
>  }
>
> +static inline void red_inc_surfaces_drawable_dependencies(DisplayChannel *display, Drawable *drawable)
> +{
> +    int x;
> +    int surface_id;
> +    RedSurface *surface;
> +
> +    for (x = 0; x < 3; ++x) {
> +        surface_id = drawable->surface_deps[x];
> +        if (surface_id == -1) {
> +            continue;
> +        }
> +        surface = &display->surfaces[surface_id];
> +        surface->refs++;
> +    }
> +}
> +
> +static void red_get_area(DisplayChannel *display, int surface_id, const SpiceRect *area,
> +                         uint8_t *dest, int dest_stride, int update)
> +{
> +    SpiceCanvas *canvas;
> +    RedSurface *surface;
> +
> +    surface = &display->surfaces[surface_id];
> +    if (update) {
> +        display_channel_draw(display, area, surface_id);
> +    }
> +
> +    canvas = surface->context.canvas;
> +    canvas->ops->read_bits(canvas, dest, dest_stride, area);
> +}
> +
> +static int display_channel_handle_self_bitmap(DisplayChannel *display, Drawable *drawable)
> +{
> +    SpiceImage *image;
> +    int32_t width;
> +    int32_t height;
> +    uint8_t *dest;
> +    int dest_stride;
> +    RedSurface *surface;
> +    int bpp;
> +    int all_set;
> +    RedDrawable *red_drawable = drawable->red_drawable;
> +
> +    if (!red_drawable->self_bitmap) {
> +        return TRUE;
> +    }
> +
> +    surface = &display->surfaces[drawable->surface_id];
> +
> +    bpp = SPICE_SURFACE_FMT_DEPTH(surface->context.format) / 8;
> +
> +    width = red_drawable->self_bitmap_area.right
> +            - red_drawable->self_bitmap_area.left;
> +    height = red_drawable->self_bitmap_area.bottom
> +            - red_drawable->self_bitmap_area.top;
> +    dest_stride = SPICE_ALIGN(width * bpp, 4);
> +
> +    image = spice_new0(SpiceImage, 1);
> +    image->descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
> +    image->descriptor.flags = 0;
> +
> +    QXL_SET_IMAGE_ID(image, QXL_IMAGE_GROUP_RED, display_channel_generate_uid(display));
> +    image->u.bitmap.flags = surface->context.top_down ? SPICE_BITMAP_FLAGS_TOP_DOWN : 0;
> +    image->u.bitmap.format = spice_bitmap_from_surface_type(surface->context.format);
> +    image->u.bitmap.stride = dest_stride;
> +    image->descriptor.width = image->u.bitmap.x = width;
> +    image->descriptor.height = image->u.bitmap.y = height;
> +    image->u.bitmap.palette = NULL;
> +
> +    dest = (uint8_t *)spice_malloc_n(height, dest_stride);
> +    image->u.bitmap.data = spice_chunks_new_linear(dest, height * dest_stride);
> +    image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE;
> +
> +    red_get_area(display, drawable->surface_id,
> +                 &red_drawable->self_bitmap_area, dest, dest_stride, TRUE);
> +
> +    /* For 32bit non-primary surfaces we need to keep any non-zero
> +       high bytes as the surface may be used as source to an alpha_blend */
> +    if (!is_primary_surface(display, drawable->surface_id) &&
> +        image->u.bitmap.format == SPICE_BITMAP_FMT_32BIT &&
> +        rgb32_data_has_alpha(width, height, dest_stride, dest, &all_set)) {
> +        if (all_set) {
> +            image->descriptor.flags |= SPICE_IMAGE_FLAGS_HIGH_BITS_SET;
> +        } else {
> +            image->u.bitmap.format = SPICE_BITMAP_FMT_RGBA;
> +        }
> +    }
> +
> +    red_drawable->self_bitmap_image = image;
> +    return TRUE;
> +}
> +
> +static inline void add_to_surface_dependency(DisplayChannel *display, int depend_on_surface_id,
> +                                             DependItem *depend_item, Drawable *drawable)
> +{
> +    RedSurface *surface;
> +
> +    if (depend_on_surface_id == -1) {
> +        depend_item->drawable = NULL;
> +        return;
> +    }
> +
> +    surface = &display->surfaces[depend_on_surface_id];
> +
> +    depend_item->drawable = drawable;
> +    ring_add(&surface->depend_on_me, &depend_item->ring_item);
> +}
> +
> +static inline int red_handle_surfaces_dependencies(DisplayChannel *display, Drawable *drawable)
> +{
> +    int x;
> +
> +    for (x = 0; x < 3; ++x) {
> +        // surface self dependency is handled by shadows in "current", or by
> +        // handle_self_bitmap
> +        if (drawable->surface_deps[x] != drawable->surface_id) {
> +            add_to_surface_dependency(display, drawable->surface_deps[x],
> +                                      &drawable->depend_items[x], drawable);
> +
> +            if (drawable->surface_deps[x] == 0) {
> +                QRegion depend_region;
> +                region_init(&depend_region);
> +                region_add(&depend_region, &drawable->red_drawable->surfaces_rects[x]);
> +                stream_detach_behind(display, &depend_region, NULL);
> +            }
> +        }
> +    }
> +
> +    return TRUE;
> +}
> +
> +static void draw_depend_on_me(DisplayChannel *display, uint32_t surface_id)
> +{
> +    RedSurface *surface;
> +    RingItem *ring_item;
> +
> +    surface = &display->surfaces[surface_id];
> +
> +    while ((ring_item = ring_get_tail(&surface->depend_on_me))) {
> +        Drawable *drawable;
> +        DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, ring_item);
> +        drawable = depended_item->drawable;
> +        display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface_id);
> +    }
> +}
> +
>  static int validate_drawable_bbox(DisplayChannel *display, RedDrawable *drawable)
>  {
>          DrawContext *context;
> @@ -980,6 +1126,45 @@ void display_channel_add_drawable(DisplayChannel *display, Drawable *drawable)
>  {
>      int success = FALSE, surface_id = drawable->surface_id;
>      RedDrawable *red_drawable = drawable->red_drawable;
> +
> +    red_drawable->mm_time = reds_get_mm_time();
> +    surface_id = drawable->surface_id;
> +
> +    display->surfaces[surface_id].refs++;
> +
> +    region_add(&drawable->tree_item.base.rgn, &red_drawable->bbox);
> +
> +    if (red_drawable->clip.type == SPICE_CLIP_TYPE_RECTS) {
> +        QRegion rgn;
> +
> +        region_init(&rgn);
> +        region_add_clip_rects(&rgn, red_drawable->clip.rects);
> +        region_and(&drawable->tree_item.base.rgn, &rgn);
> +        region_destroy(&rgn);
> +    }
> +
> +    /*
> +        surface->refs is affected by a drawable (that is
> +        dependent on the surface) as long as the drawable is alive.
> +        However, surface->depend_on_me is affected by a drawable only
> +        as long as it is in the current tree (hasn't been rendered yet).
> +    */
> +    red_inc_surfaces_drawable_dependencies(display, drawable);
> +
> +    if (region_is_empty(&drawable->tree_item.base.rgn)) {
> +        return;
> +    }
> +
> +    if (!display_channel_handle_self_bitmap(display, drawable)) {
> +        return;
> +    }
> +
> +    draw_depend_on_me(display, surface_id);
> +
> +    if (!red_handle_surfaces_dependencies(display, drawable)) {
> +        return;
> +    }
> +
>      Ring *ring = &display->surfaces[surface_id].current;
>
>      if (has_shadow(red_drawable)) {
> diff --git a/server/display-channel.h b/server/display-channel.h
> index 6c0862c..83b50ca 100644
> --- a/server/display-channel.h
> +++ b/server/display-channel.h
> @@ -442,19 +442,4 @@ static inline void region_add_clip_rects(QRegion *rgn, SpiceClipRects *data)
>      }
>  }
>
> -static inline void draw_depend_on_me(DisplayChannel *display, uint32_t surface_id)
> -{
> -    RedSurface *surface;
> -    RingItem *ring_item;
> -
> -    surface = &display->surfaces[surface_id];
> -
> -    while ((ring_item = ring_get_tail(&surface->depend_on_me))) {
> -        Drawable *drawable;
> -        DependItem *depended_item = SPICE_CONTAINEROF(ring_item, DependItem, ring_item);
> -        drawable = depended_item->drawable;
> -        display_channel_draw(display, &drawable->red_drawable->bbox, drawable->surface_id);
> -    }
> -}
> -
>  #endif /* DISPLAY_CHANNEL_H_ */
> diff --git a/server/red_worker.c b/server/red_worker.c
> index ff288e4..329c2ef 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -165,143 +165,10 @@ void red_drawable_unref(RedWorker *worker, RedDrawable *red_drawable,
>      free(red_drawable);
>  }
>
> -static void red_get_area(DisplayChannel *display, int surface_id, const SpiceRect *area,
> -                         uint8_t *dest, int dest_stride, int update)
> -{
> -    SpiceCanvas *canvas;
> -    RedSurface *surface;
> -
> -    surface = &display->surfaces[surface_id];
> -    if (update) {
> -        display_channel_draw(display, area, surface_id);
> -    }
> -
> -    canvas = surface->context.canvas;
> -    canvas->ops->read_bits(canvas, dest, dest_stride, area);
> -}
> -
> -static inline int red_handle_self_bitmap(RedWorker *worker, Drawable *drawable)
> -{
> -    DisplayChannel *display = worker->display_channel;
> -    SpiceImage *image;
> -    int32_t width;
> -    int32_t height;
> -    uint8_t *dest;
> -    int dest_stride;
> -    RedSurface *surface;
> -    int bpp;
> -    int all_set;
> -    RedDrawable *red_drawable = drawable->red_drawable;
> -
> -    if (!red_drawable->self_bitmap) {
> -        return TRUE;
> -    }
> -
> -    surface = &display->surfaces[drawable->surface_id];
> -
> -    bpp = SPICE_SURFACE_FMT_DEPTH(surface->context.format) / 8;
> -
> -    width = red_drawable->self_bitmap_area.right
> -            - red_drawable->self_bitmap_area.left;
> -    height = red_drawable->self_bitmap_area.bottom
> -            - red_drawable->self_bitmap_area.top;
> -    dest_stride = SPICE_ALIGN(width * bpp, 4);
> -
> -    image = spice_new0(SpiceImage, 1);
> -    image->descriptor.type = SPICE_IMAGE_TYPE_BITMAP;
> -    image->descriptor.flags = 0;
> -
> -    QXL_SET_IMAGE_ID(image, QXL_IMAGE_GROUP_RED, display_channel_generate_uid(display));
> -    image->u.bitmap.flags = surface->context.top_down ? SPICE_BITMAP_FLAGS_TOP_DOWN : 0;
> -    image->u.bitmap.format = spice_bitmap_from_surface_type(surface->context.format);
> -    image->u.bitmap.stride = dest_stride;
> -    image->descriptor.width = image->u.bitmap.x = width;
> -    image->descriptor.height = image->u.bitmap.y = height;
> -    image->u.bitmap.palette = NULL;
> -
> -    dest = (uint8_t *)spice_malloc_n(height, dest_stride);
> -    image->u.bitmap.data = spice_chunks_new_linear(dest, height * dest_stride);
> -    image->u.bitmap.data->flags |= SPICE_CHUNKS_FLAGS_FREE;
> -
> -    red_get_area(display, drawable->surface_id,
> -                 &red_drawable->self_bitmap_area, dest, dest_stride, TRUE);
> -
> -    /* For 32bit non-primary surfaces we need to keep any non-zero
> -       high bytes as the surface may be used as source to an alpha_blend */
> -    if (!is_primary_surface(display, drawable->surface_id) &&
> -        image->u.bitmap.format == SPICE_BITMAP_FMT_32BIT &&
> -        rgb32_data_has_alpha(width, height, dest_stride, dest, &all_set)) {
> -        if (all_set) {
> -            image->descriptor.flags |= SPICE_IMAGE_FLAGS_HIGH_BITS_SET;
> -        } else {
> -            image->u.bitmap.format = SPICE_BITMAP_FMT_RGBA;
> -        }
> -    }
> -
> -    red_drawable->self_bitmap_image = image;
> -    return TRUE;
> -}
> -
> -static inline void add_to_surface_dependency(DisplayChannel *display, int depend_on_surface_id,
> -                                             DependItem *depend_item, Drawable *drawable)
> -{
> -    RedSurface *surface;
> -
> -    if (depend_on_surface_id == -1) {
> -        depend_item->drawable = NULL;
> -        return;
> -    }
> -
> -    surface = &display->surfaces[depend_on_surface_id];
> -
> -    depend_item->drawable = drawable;
> -    ring_add(&surface->depend_on_me, &depend_item->ring_item);
> -}
> -
> -static inline int red_handle_surfaces_dependencies(DisplayChannel *display, Drawable *drawable)
> -{
> -    int x;
> -
> -    for (x = 0; x < 3; ++x) {
> -        // surface self dependency is handled by shadows in "current", or by
> -        // handle_self_bitmap
> -        if (drawable->surface_deps[x] != drawable->surface_id) {
> -            add_to_surface_dependency(display, drawable->surface_deps[x],
> -                                      &drawable->depend_items[x], drawable);
> -
> -            if (drawable->surface_deps[x] == 0) {
> -                QRegion depend_region;
> -                region_init(&depend_region);
> -                region_add(&depend_region, &drawable->red_drawable->surfaces_rects[x]);
> -                stream_detach_behind(display, &depend_region, NULL);
> -            }
> -        }
> -    }
> -
> -    return TRUE;
> -}
> -
> -static inline void red_inc_surfaces_drawable_dependencies(DisplayChannel *display, Drawable *drawable)
> -{
> -    int x;
> -    int surface_id;
> -    RedSurface *surface;
> -
> -    for (x = 0; x < 3; ++x) {
> -        surface_id = drawable->surface_deps[x];
> -        if (surface_id == -1) {
> -            continue;
> -        }
> -        surface = &display->surfaces[surface_id];
> -        surface->refs++;
> -    }
> -}
> -
>  static inline void red_process_draw(RedWorker *worker, RedDrawable *red_drawable,
>                                      uint32_t group_id)
>  {
>      DisplayChannel *display = worker->display_channel;
> -    int surface_id;
>      Drawable *drawable =
>          display_channel_get_drawable(display, red_drawable->effect, red_drawable, group_id,
>                                       worker->process_commands_generation);
> @@ -310,47 +177,8 @@ static inline void red_process_draw(RedWorker *worker, RedDrawable *red_drawable
>          return;
>      }
>
> -    red_drawable->mm_time = reds_get_mm_time();
> -    surface_id = drawable->surface_id;
> -
> -    display->surfaces[surface_id].refs++;
> -
> -    region_add(&drawable->tree_item.base.rgn, &red_drawable->bbox);
> -
> -    if (red_drawable->clip.type == SPICE_CLIP_TYPE_RECTS) {
> -        QRegion rgn;
> -
> -        region_init(&rgn);
> -        region_add_clip_rects(&rgn, red_drawable->clip.rects);
> -        region_and(&drawable->tree_item.base.rgn, &rgn);
> -        region_destroy(&rgn);
> -    }
> -
> -    /*
> -        surface->refs is affected by a drawable (that is
> -        dependent on the surface) as long as the drawable is alive.
> -        However, surface->depend_on_me is affected by a drawable only
> -        as long as it is in the current tree (hasn't been rendered yet).
> -    */
> -    red_inc_surfaces_drawable_dependencies(worker->display_channel, drawable);
> -
> -    if (region_is_empty(&drawable->tree_item.base.rgn)) {
> -        goto cleanup;
> -    }
> -
> -    if (!red_handle_self_bitmap(worker, drawable)) {
> -        goto cleanup;
> -    }
> -
> -    draw_depend_on_me(display, surface_id);
> -
> -    if (!red_handle_surfaces_dependencies(worker->display_channel, drawable)) {
> -        goto cleanup;
> -    }
> -
>      display_channel_add_drawable(worker->display_channel, drawable);
>
> -cleanup:
>      display_channel_drawable_unref(display, drawable);
>  }
>
> --
> 2.4.3
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

Acked-by: Fabiano Fidêncio <fidencio at redhat.com>


More information about the Spice-devel mailing list