On Wed, Nov 25, 2015 at 4:27 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> > > --- > server/display-channel.c | 229 +++++++++++++++++++++++++++++++++++++++-- > server/display-channel.h | 23 ++--- > server/red_worker.c | 258 +++-------------------------------------------- > 3 files changed, 243 insertions(+), 267 deletions(-) > > diff --git a/server/display-channel.c b/server/display-channel.c > index 4a533db..ded306b 100644 > --- a/server/display-channel.c > +++ b/server/display-channel.c > @@ -20,6 +20,13 @@ > > #include "display-channel.h" > > +uint32_t generate_uid(DisplayChannel *display) > +{ > + spice_return_val_if_fail(display != NULL, 0); > + > + return ++display->bits_unique; > +} > + > static stat_time_t display_channel_stat_now(DisplayChannel *display) > { > #ifdef RED_WORKER_STAT > @@ -832,26 +839,236 @@ void display_channel_print_stats(DisplayChannel *display) > #endif > } > > -void display_channel_add_drawable(DisplayChannel *display, Drawable *drawable) > +static void drawable_ref_surface_deps(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 surface_read_bits(DisplayChannel *display, int surface_id, > + const SpiceRect *area, uint8_t *dest, int dest_stride) > +{ > + SpiceCanvas *canvas; > + RedSurface *surface = &display->surfaces[surface_id]; > + > + canvas = surface->context.canvas; > + canvas->ops->read_bits(canvas, dest, dest_stride, area); > +} > + > +static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable) > { > - int success = FALSE, surface_id = drawable->surface_id; > RedDrawable *red_drawable = drawable->red_drawable; > - Ring *ring = &display->surfaces[surface_id].current; > + SpiceImage *image; > + int32_t width; > + int32_t height; > + uint8_t *dest; > + int dest_stride; > + RedSurface *surface; > + int bpp; > + int all_set; > + > + 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, 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; > + > + display_channel_draw(display, &red_drawable->self_bitmap_area, drawable->surface_id); > + surface_read_bits(display, drawable->surface_id, > + &red_drawable->self_bitmap_area, dest, dest_stride); > + > + /* 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; > +} > + > +static void surface_add_reverse_dependency(DisplayChannel *display, int surface_id, > + DependItem *depend_item, Drawable *drawable) > +{ > + RedSurface *surface; > + > + if (surface_id == -1) { > + depend_item->drawable = NULL; > + return; > + } > + > + surface = &display->surfaces[surface_id]; > + > + depend_item->drawable = drawable; > + ring_add(&surface->depend_on_me, &depend_item->ring_item); > +} > + > +static int handle_surface_deps(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) { > + surface_add_reverse_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]); > + detach_streams_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; > + uint32_t surface_id = drawable->surface_id; > + > + /* surface_id must be validated before calling into > + * validate_drawable_bbox > + */ > + if (!validate_surface(display, drawable->surface_id)) { > + return FALSE; > + } > + context = &display->surfaces[surface_id].context; > + > + if (drawable->bbox.top < 0) > + return FALSE; > + if (drawable->bbox.left < 0) > + return FALSE; > + if (drawable->bbox.bottom < 0) > + return FALSE; > + if (drawable->bbox.right < 0) > + return FALSE; > + if (drawable->bbox.bottom > context->height) > + return FALSE; > + if (drawable->bbox.right > context->width) > + return FALSE; > > + return TRUE; > +} > + > + > +int display_channel_add_drawable(DisplayChannel *display, Drawable *drawable, RedDrawable *red_drawable) > +{ > + int surface_id, x; > + > + drawable->red_drawable = red_drawable_ref(red_drawable); > + drawable->tree_item.effect = red_drawable->effect; > + surface_id = drawable->surface_id = red_drawable->surface_id; > + if (!validate_drawable_bbox(display, red_drawable)) > + return FALSE; > + // FIXME here can leak if after we return! > + display->surfaces[surface_id].refs++; > + red_drawable->mm_time = reds_get_mm_time(); > + > + for (x = 0; x < 3; ++x) { > + drawable->surface_deps[x] = red_drawable->surface_deps[x]; > + if (drawable->surface_deps[x] != -1 > + && !validate_surface(display, drawable->surface_deps[x])) > + return FALSE; > + } > + > + 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); > + } > + > + if (region_is_empty(&drawable->tree_item.base.rgn)) > + return TRUE; > + > + /* > + 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). > + */ > + drawable_ref_surface_deps(display, drawable); > + > + if (red_drawable->self_bitmap) > + handle_self_bitmap(display, drawable); > + > + draw_depend_on_me(display, surface_id); > + > + if (!handle_surface_deps(display, drawable)) > + return FALSE; > + > + Ring *ring = &display->surfaces[surface_id].current; > + int add_to_pipe = FALSE; > if (has_shadow(red_drawable)) { > - success = current_add_with_shadow(display, ring, drawable); > + add_to_pipe = current_add_with_shadow(display, ring, drawable); > } else { > drawable->streamable = drawable_can_stream(display, drawable); > - success = current_add(display, ring, drawable); > + add_to_pipe = current_add(display, ring, drawable); > } > > - if (success) > + if (add_to_pipe) > pipes_add_drawable(display, drawable); > > #ifdef RED_WORKER_STAT > if ((++display->add_count % 100) == 0) > display_channel_print_stats(display); > #endif > + > + return TRUE; > } > > int display_channel_wait_for_migrate_data(DisplayChannel *display) > diff --git a/server/display-channel.h b/server/display-channel.h > index 0d79463..4a79700 100644 > --- a/server/display-channel.h > +++ b/server/display-channel.h > @@ -165,6 +165,7 @@ struct _Drawable { > > struct DisplayChannel { > CommonChannel common; // Must be the first thing > + uint32_t bits_unique; > > MonitorsConfig *monitors_config; > > @@ -286,8 +287,9 @@ void display_channel_surface_unref (DisplayCha > uint32_t surface_id); > bool display_channel_surface_has_canvas (DisplayChannel *display, > uint32_t surface_id); > -void display_channel_add_drawable (DisplayChannel *display, > - Drawable *drawable); > +int display_channel_add_drawable (DisplayChannel *display, > + Drawable *drawable, > + RedDrawable *red_drawable); > void display_channel_current_flush (DisplayChannel *display, > int surface_id); > int display_channel_wait_for_migrate_data (DisplayChannel *display); > @@ -299,6 +301,7 @@ void display_channel_destroy_surface_wait (DisplayCha > void display_channel_destroy_surfaces (DisplayChannel *display); > void display_channel_destroy_surface (DisplayChannel *display, > uint32_t surface_id); > +uint32_t display_channel_generate_uid (DisplayChannel *display); > > static inline int validate_surface(DisplayChannel *display, uint32_t surface_id) > { > @@ -430,21 +433,7 @@ 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); > - } > -} > - > +uint32_t generate_uid(DisplayChannel *display); > void current_remove_drawable(DisplayChannel *display, Drawable *item); > void red_pipes_remove_drawable(Drawable *drawable); > void current_remove(DisplayChannel *display, TreeItem *item); > diff --git a/server/red_worker.c b/server/red_worker.c > index 6ea7cd5..84048fe 100644 > --- a/server/red_worker.c > +++ b/server/red_worker.c > @@ -99,8 +99,6 @@ struct RedWorker { > CursorChannel *cursor_channel; > uint32_t cursor_poll_tries; > > - uint32_t bits_unique; > - > RedMemSlotInfo mem_slots; > > SpiceImageCompression image_compression; > @@ -142,35 +140,6 @@ QXLInstance* red_worker_get_qxl(RedWorker *worker) > return worker->qxl; > } > > -static int validate_drawable_bbox(DisplayChannel *display, RedDrawable *drawable) > -{ > - DrawContext *context; > - uint32_t surface_id = drawable->surface_id; > - > - /* surface_id must be validated before calling into > - * validate_drawable_bbox > - */ > - if (!validate_surface(display, drawable->surface_id)) { > - return FALSE; > - } > - context = &display->surfaces[surface_id].context; > - > - if (drawable->bbox.top < 0) > - return FALSE; > - if (drawable->bbox.left < 0) > - return FALSE; > - if (drawable->bbox.bottom < 0) > - return FALSE; > - if (drawable->bbox.right < 0) > - return FALSE; > - if (drawable->bbox.bottom > context->height) > - return FALSE; > - if (drawable->bbox.right > context->width) > - return FALSE; > - > - return TRUE; > -} > - > static int display_is_connected(RedWorker *worker) > { > return (worker->display_channel && red_channel_is_connected( > @@ -489,227 +458,28 @@ static void display_channel_streams_timeout(DisplayChannel *display) > } > } > > -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, ++worker->bits_unique); > - 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 Drawable *get_drawable(RedWorker *worker, uint8_t effect, RedDrawable *red_drawable, > - uint32_t group_id) > +static void red_process_draw(RedWorker *worker, RedDrawable *red_drawable, > + uint32_t group_id) > { > DisplayChannel *display = worker->display_channel; > Drawable *drawable; > - int x; > - > - if (!validate_drawable_bbox(display, red_drawable)) { > - return NULL; > - } > - for (x = 0; x < 3; ++x) { > - if (red_drawable->surface_deps[x] != -1 > - && !validate_surface(display, red_drawable->surface_deps[x])) { > - return NULL; > - } > - } > - > - drawable = display_channel_drawable_try_new(display, group_id, worker->process_commands_generation); > - if (!drawable) { > - return NULL; > - } > - > - drawable->tree_item.effect = effect; > - drawable->red_drawable = red_drawable_ref(red_drawable); > - > - drawable->surface_id = red_drawable->surface_id; > - memcpy(drawable->surface_deps, red_drawable->surface_deps, sizeof(drawable->surface_deps)); > - > - return drawable; > -} > - > -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]); > - detach_streams_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 = get_drawable(worker, red_drawable->effect, red_drawable, group_id); > + bool success = FALSE; > > + drawable = display_channel_drawable_try_new(display, group_id, > + worker->process_commands_generation); > if (!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); > + success = display_channel_add_drawable(worker->display_channel, drawable, red_drawable); > + spice_warn_if_fail(success); > > - 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); > } > > > -static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface, > - uint32_t group_id, int loadvm) > +static void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface, > + uint32_t group_id, int loadvm) > { > DisplayChannel *display = worker->display_channel; > uint32_t surface_id; > @@ -2965,7 +2735,7 @@ static void display_channel_marshall_reset_cache(RedChannelClient *rcc, > static void red_marshall_image(RedChannelClient *rcc, SpiceMarshaller *m, ImageItem *item) > { > DisplayChannelClient *dcc = RCC_TO_DCC(rcc); > - DisplayChannel *display_channel = DCC_TO_DC(dcc); > + DisplayChannel *display = DCC_TO_DC(dcc); > SpiceImage red_image; > RedWorker *worker; > SpiceBitmap bitmap; > @@ -2979,10 +2749,10 @@ static void red_marshall_image(RedChannelClient *rcc, SpiceMarshaller *m, ImageI > SpiceMarshaller *src_bitmap_out, *mask_bitmap_out; > SpiceMarshaller *bitmap_palette_out, *lzplt_palette_out; > > - spice_assert(rcc && display_channel && item); > - worker = display_channel->common.worker; > + spice_assert(rcc && display && item); > + worker = display->common.worker; > > - QXL_SET_IMAGE_ID(&red_image, QXL_IMAGE_GROUP_RED, ++worker->bits_unique); > + QXL_SET_IMAGE_ID(&red_image, QXL_IMAGE_GROUP_RED, generate_uid(display)); > red_image.descriptor.type = SPICE_IMAGE_TYPE_BITMAP; > red_image.descriptor.flags = item->image_flags; > red_image.descriptor.width = item->width; > @@ -3039,7 +2809,7 @@ static void red_marshall_image(RedChannelClient *rcc, SpiceMarshaller *m, ImageI > grad_level = bitmap_get_graduality_level(&bitmap); > if (grad_level == BITMAP_GRADUAL_HIGH) { > // if we use lz for alpha, the stride can't be extra > - lossy_comp = display_channel->enable_jpeg && item->can_lossy; > + lossy_comp = display->enable_jpeg && item->can_lossy; > quic_comp = TRUE; > } > } > -- > 2.4.3 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel Acked-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx> -- Fabiano Fidêncio _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel