[Spice-devel] [PATCH 07/19] display: move more logic in add_drawable()
Frediano Ziglio
fziglio at redhat.com
Wed Nov 25 07:27:31 PST 2015
From: Marc-André Lureau <marcandre.lureau at gmail.com>
---
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
More information about the Spice-devel
mailing list