[Spice-devel] [PATCH v2 1/2] display-channel: Join drawables to improve rhel7 behaviour
Frediano Ziglio
fziglio at redhat.com
Thu Mar 2 11:34:58 UTC 2017
Due to the way RHEL7 works the images came out from guest using multiple
commands. This increase the commands to the client and cause the
video code to create and handle multiple streams creating some
visual glitches.
This patch attempt to detect and join the multiple commands to
avoid these issues.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
server/display-channel-private.h | 2 +-
server/display-channel.c | 173 +++++++++++++++++++++++++++++++-
server/red-parse-qxl.h | 1 +-
server/red-worker.c | 14 +--
4 files changed, 183 insertions(+), 7 deletions(-)
diff --git a/server/display-channel-private.h b/server/display-channel-private.h
index da807d1..62e03b6 100644
--- a/server/display-channel-private.h
+++ b/server/display-channel-private.h
@@ -69,6 +69,8 @@ struct DisplayChannelPrivate
int gl_draw_async_count;
+ RedDrawable *joinable_drawable;
+
/* TODO: some day unify this, make it more runtime.. */
stat_info_t add_stat;
stat_info_t exclude_stat;
diff --git a/server/display-channel.c b/server/display-channel.c
index fa2b281..37d1703 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1175,8 +1175,147 @@ static void display_channel_add_drawable(DisplayChannel *display, Drawable *draw
#endif
}
-void display_channel_process_draw(DisplayChannel *display, RedDrawable *red_drawable,
- uint32_t process_commands_generation)
+/* Check that a given drawable it's a simple copy that can be
+ * possibly be joined to next one.
+ * This is used to undo some engine which split images into
+ * chunks causing more commands and creating multiple streams.
+ * One example is RHEL 7.
+ */
+static gboolean red_drawable_joinable(const RedDrawable *drawable)
+{
+ /* these 3 initial tests are arranged to minimize checks as
+ * they are in reverse order of occurrences */
+ if (drawable->clip.type != SPICE_CLIP_TYPE_NONE) {
+ return FALSE;
+ }
+ if (drawable->type != QXL_DRAW_COPY) {
+ return FALSE;
+ }
+ if (drawable->effect != QXL_EFFECT_OPAQUE) {
+ return FALSE;
+ }
+ if (drawable->self_bitmap) {
+ return FALSE;
+ }
+ if (drawable->surface_deps[0] != -1 ||
+ drawable->surface_deps[1] != -1 ||
+ drawable->surface_deps[2] != -1) {
+ return FALSE;
+ }
+
+ const SpiceCopy *copy = &drawable->u.copy;
+ if (copy->src_bitmap == NULL) {
+ return FALSE;
+ }
+ if (copy->rop_descriptor != SPICE_ROPD_OP_PUT) {
+ return FALSE;
+ }
+ if (copy->mask.bitmap != NULL) {
+ return FALSE;
+ }
+
+ const SpiceImage *image = copy->src_bitmap;
+ if (image->descriptor.type != SPICE_IMAGE_TYPE_BITMAP) {
+ return FALSE;
+ }
+ const SpiceBitmap *bitmap = &image->u.bitmap;
+ if (bitmap->format != SPICE_BITMAP_FMT_RGBA && bitmap->format != SPICE_BITMAP_FMT_32BIT) {
+ return FALSE;
+ }
+ if (bitmap->flags != SPICE_BITMAP_FLAGS_TOP_DOWN) {
+ return FALSE;
+ }
+ if (bitmap->palette != NULL) {
+ return FALSE;
+ }
+ if (bitmap->data == NULL) {
+ return FALSE;
+ }
+ if (bitmap->data->flags & SPICE_CHUNKS_FLAGS_UNSTABLE) {
+ return FALSE;
+ }
+ if (bitmap->x != image->descriptor.width ||
+ bitmap->y != image->descriptor.height) {
+ return FALSE;
+ }
+ /* area should specify all image */
+ if (copy->src_area.left != 0 || copy->src_area.top != 0 ||
+ copy->src_area.right != bitmap->x || copy->src_area.bottom != bitmap->y) {
+ return FALSE;
+ }
+ if (drawable->bbox.right - drawable->bbox.left != bitmap->x ||
+ drawable->bbox.bottom - drawable->bbox.top != bitmap->y) {
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+static gboolean red_drawable_can_join(const RedDrawable *first, const RedDrawable *second)
+{
+ if (!red_drawable_joinable(first) || !red_drawable_joinable(second)) {
+ return FALSE;
+ }
+ if (first->surface_id != second->surface_id) {
+ return FALSE;
+ }
+ if (first->u.copy.src_bitmap->u.bitmap.format != second->u.copy.src_bitmap->u.bitmap.format) {
+ return FALSE;
+ }
+ // they must have the same width
+ if (first->u.copy.src_bitmap->u.bitmap.x != second->u.copy.src_bitmap->u.bitmap.x ||
+ first->u.copy.src_bitmap->u.bitmap.stride != second->u.copy.src_bitmap->u.bitmap.stride) {
+ return FALSE;
+ }
+ // check that second is exactly under the first
+ if (first->bbox.left != second->bbox.left) {
+ return FALSE;
+ }
+ if (first->bbox.bottom != second->bbox.top) {
+ return FALSE;
+ }
+ // check we can join the chunks
+ if (first->u.copy.src_bitmap->u.bitmap.data->flags !=
+ second->u.copy.src_bitmap->u.bitmap.data->flags) {
+ return FALSE;
+ }
+ return TRUE;
+}
+
+static SpiceChunks *chunks_join(const SpiceChunks *first, const SpiceChunks *second)
+{
+ // TODO use realloc to optimize
+ SpiceChunks *new_chunks = spice_chunks_new(first->num_chunks + second->num_chunks);
+ new_chunks->flags = first->flags;
+ new_chunks->data_size = first->data_size + second->data_size;
+ memcpy(new_chunks->chunk, first->chunk, sizeof(first->chunk[0]) * first->num_chunks);
+ memcpy(new_chunks->chunk + first->num_chunks, second->chunk, sizeof(second->chunk[0]) * second->num_chunks);
+ return new_chunks;
+}
+
+static RedDrawable *red_drawable_join(RedDrawable *first, RedDrawable *second)
+{
+ uint32_t first_height = first->u.copy.src_bitmap->u.bitmap.y;
+ second->u.copy.src_bitmap->descriptor.flags &= ~SPICE_IMAGE_FLAGS_CACHE_ME;
+ second->bbox.top = first->bbox.top;
+ second->u.copy.src_bitmap->descriptor.height += first_height;
+ second->u.copy.src_bitmap->u.bitmap.y += first_height;
+ second->u.copy.src_area.bottom += first_height;
+ // join chunks
+ SpiceChunks *new_chunks = chunks_join(first->u.copy.src_bitmap->u.bitmap.data, second->u.copy.src_bitmap->u.bitmap.data);
+ // prevent to free chunks copied in new structure
+ second->u.copy.src_bitmap->u.bitmap.data->num_chunks = 0;
+ first->u.copy.src_bitmap->u.bitmap.data->num_chunks = 0;
+ // replace second one
+ spice_chunks_destroy(second->u.copy.src_bitmap->u.bitmap.data);
+ second->u.copy.src_bitmap->u.bitmap.data = new_chunks;
+ second->joined = first;
+ return second;
+}
+
+static void
+display_channel_process_draw_single(DisplayChannel *display, RedDrawable *red_drawable,
+ uint32_t process_commands_generation)
{
Drawable *drawable =
display_channel_get_drawable(display, red_drawable->effect, red_drawable,
@@ -1191,6 +1330,36 @@ void display_channel_process_draw(DisplayChannel *display, RedDrawable *red_draw
drawable_unref(drawable);
}
+void display_channel_process_draw(DisplayChannel *display, RedDrawable *red_drawable,
+ uint32_t process_commands_generation)
+{
+ if (!red_drawable_joinable(red_drawable)) {
+ // not joinable, process all
+ if (display->priv->joinable_drawable) {
+ display_channel_process_draw_single(display, display->priv->joinable_drawable,
+ process_commands_generation);
+ red_drawable_unref(display->priv->joinable_drawable);
+ display->priv->joinable_drawable = NULL;
+ }
+ display_channel_process_draw_single(display, red_drawable, process_commands_generation);
+ return;
+ }
+
+ if (display->priv->joinable_drawable == NULL) {
+ // try to join with next one
+ } else if (red_drawable_can_join(display->priv->joinable_drawable, red_drawable)) {
+ red_drawable = red_drawable_join(display->priv->joinable_drawable, red_drawable);
+ } else {
+ // they can't be joined
+ display_channel_process_draw_single(display, display->priv->joinable_drawable,
+ process_commands_generation);
+ red_drawable_unref(display->priv->joinable_drawable);
+ }
+ // try to join with next one
+ red_drawable_ref(red_drawable);
+ display->priv->joinable_drawable = red_drawable;
+}
+
int display_channel_wait_for_migrate_data(DisplayChannel *display)
{
uint64_t end_time = spice_get_monotonic_time_ns() + DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT;
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 86a2d93..56cc906 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -57,6 +57,7 @@ typedef struct RedDrawable {
SpiceWhiteness whiteness;
SpiceComposite composite;
} u;
+ struct RedDrawable *joined;
} RedDrawable;
static inline RedDrawable *red_drawable_ref(RedDrawable *drawable)
diff --git a/server/red-worker.c b/server/red-worker.c
index 8735cd1..b0d2955 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -94,12 +94,16 @@ static int display_is_connected(RedWorker *worker)
void red_drawable_unref(RedDrawable *red_drawable)
{
- if (--red_drawable->refs) {
- return;
+ while (red_drawable) {
+ if (--red_drawable->refs) {
+ return;
+ }
+ red_qxl_release_resource(red_drawable->qxl, red_drawable->release_info_ext);
+ red_put_drawable(red_drawable);
+ RedDrawable *next = red_drawable->joined;
+ free(red_drawable);
+ red_drawable = next;
}
- red_qxl_release_resource(red_drawable->qxl, red_drawable->release_info_ext);
- red_put_drawable(red_drawable);
- free(red_drawable);
}
static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *ext)
--
git-series 0.9.1
More information about the Spice-devel
mailing list