[Spice-devel] [PATCH 3/3] display: move more logic in add_drawable()
Frediano Ziglio
fziglio at redhat.com
Fri Nov 27 04:58:49 PST 2015
From: Marc-André Lureau <marcandre.lureau at gmail.com>
Acked-by: Fabiano Fidêncio <fidencio at redhat.com>
fziglio ???
---
server/display-channel.c | 129 +++++++++++++++++------------------------------
server/display-channel.h | 10 ++--
server/red_worker.c | 18 ++++---
3 files changed, 60 insertions(+), 97 deletions(-)
diff --git a/server/display-channel.c b/server/display-channel.c
index 0b4415b..d8ab849 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -916,7 +916,7 @@ void display_channel_print_stats(DisplayChannel *display)
#endif
}
-static inline void red_inc_surfaces_drawable_dependencies(DisplayChannel *display, Drawable *drawable)
+static void drawable_ref_surface_deps(DisplayChannel *display, Drawable *drawable)
{
int x;
int surface_id;
@@ -932,23 +932,19 @@ static inline void red_inc_surfaces_drawable_dependencies(DisplayChannel *displa
}
}
-static void red_get_area(DisplayChannel *display, int surface_id, const SpiceRect *area,
- uint8_t *dest, int dest_stride, int update)
+static void surface_read_bits(DisplayChannel *display, int surface_id,
+ const SpiceRect *area, uint8_t *dest, int dest_stride)
{
SpiceCanvas *canvas;
- RedSurface *surface;
-
- surface = &display->surfaces[surface_id];
- if (update) {
- display_channel_draw(display, area, surface_id);
- }
+ RedSurface *surface = &display->surfaces[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)
+static void handle_self_bitmap(DisplayChannel *display, Drawable *drawable)
{
+ RedDrawable *red_drawable = drawable->red_drawable;
SpiceImage *image;
int32_t width;
int32_t height;
@@ -957,26 +953,17 @@ static int display_channel_handle_self_bitmap(DisplayChannel *display, Drawable
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;
+ 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);
@@ -989,8 +976,9 @@ static int display_channel_handle_self_bitmap(DisplayChannel *display, Drawable
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);
+ 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 */
@@ -1005,26 +993,25 @@ static int display_channel_handle_self_bitmap(DisplayChannel *display, Drawable
}
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)
+static void surface_add_reverse_dependency(DisplayChannel *display, int surface_id,
+ DependItem *depend_item, Drawable *drawable)
{
RedSurface *surface;
- if (depend_on_surface_id == -1) {
+ if (surface_id == -1) {
depend_item->drawable = NULL;
return;
}
- surface = &display->surfaces[depend_on_surface_id];
+ surface = &display->surfaces[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)
+static int handle_surface_deps(DisplayChannel *display, Drawable *drawable)
{
int x;
@@ -1032,8 +1019,8 @@ static inline int red_handle_surfaces_dependencies(DisplayChannel *display, Draw
// 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);
+ surface_add_reverse_dependency(display, drawable->surface_deps[x],
+ &drawable->depend_items[x], drawable);
if (drawable->surface_deps[x] == 0) {
QRegion depend_region;
@@ -1091,49 +1078,28 @@ static int validate_drawable_bbox(DisplayChannel *display, RedDrawable *drawable
return TRUE;
}
-Drawable *display_channel_get_drawable(DisplayChannel *display, uint8_t effect,
- RedDrawable *red_drawable, uint32_t group_id,
- uint32_t process_commands_generation)
-{
- 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, 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;
-}
-
-void display_channel_add_drawable(DisplayChannel *display, Drawable *drawable)
+int display_channel_add_drawable(DisplayChannel *display, Drawable *drawable, RedDrawable *red_drawable)
{
- int success = FALSE, surface_id = drawable->surface_id;
- RedDrawable *red_drawable = drawable->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();
- surface_id = drawable->surface_id;
- display->surfaces[surface_id].refs++;
+ 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;
@@ -1143,44 +1109,43 @@ void display_channel_add_drawable(DisplayChannel *display, Drawable *drawable)
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).
*/
- red_inc_surfaces_drawable_dependencies(display, drawable);
+ drawable_ref_surface_deps(display, drawable);
- if (region_is_empty(&drawable->tree_item.base.rgn)) {
- return;
- }
-
- if (!display_channel_handle_self_bitmap(display, drawable)) {
- return;
- }
+ if (red_drawable->self_bitmap)
+ handle_self_bitmap(display, drawable);
draw_depend_on_me(display, surface_id);
- if (!red_handle_surfaces_dependencies(display, drawable)) {
- return;
- }
+ 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 195004d..2ac8173 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -286,8 +286,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);
@@ -300,11 +301,6 @@ void display_channel_destroy_surfaces (DisplayCha
void display_channel_destroy_surface (DisplayChannel *display,
uint32_t surface_id);
uint32_t display_channel_generate_uid (DisplayChannel *display);
-Drawable * display_channel_get_drawable (DisplayChannel *display,
- uint8_t effect,
- RedDrawable *red_drawable,
- uint32_t group_id,
- uint32_t process_commands_generation);
static inline int validate_surface(DisplayChannel *display, uint32_t surface_id)
{
diff --git a/server/red_worker.c b/server/red_worker.c
index ee26b63..f943898 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -172,26 +172,28 @@ void red_drawable_unref(RedWorker *worker, RedDrawable *red_drawable,
free(red_drawable);
}
-static inline void red_process_draw(RedWorker *worker, 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 =
- display_channel_get_drawable(display, red_drawable->effect, red_drawable, group_id,
- worker->process_commands_generation);
+ Drawable *drawable;
+ bool success = FALSE;
+ drawable = display_channel_drawable_try_new(display, group_id,
+ worker->process_commands_generation);
if (!drawable) {
return;
}
- display_channel_add_drawable(worker->display_channel, drawable);
+ success = display_channel_add_drawable(worker->display_channel, drawable, red_drawable);
+ spice_warn_if_fail(success);
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;
--
2.4.3
More information about the Spice-devel
mailing list