[Spice-devel] [PATCH spice-server 1/2] RFC: Join drawables to improve rhel7 behavior
Frediano Ziglio
fziglio at redhat.com
Tue Dec 6 12:28:32 UTC 2016
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/red-parse-qxl.h | 1 +
server/red-worker.c | 181 +++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 175 insertions(+), 7 deletions(-)
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 79e459f..b841782 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -84,6 +84,8 @@ struct RedWorker {
RedRecord *record;
GMainLoop *loop;
+
+ RedDrawable *joinable_drawable;
};
static int display_is_connected(RedWorker *worker)
@@ -94,12 +96,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 int red_process_cursor(RedWorker *worker, int *ring_is_empty)
@@ -163,6 +169,144 @@ static RedDrawable *red_drawable_new(QXLInstance *qxl)
return red;
}
+/* 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 int red_process_display(RedWorker *worker, int *ring_is_empty)
{
QXLCommandExt ext_cmd;
@@ -201,8 +345,31 @@ static int red_process_display(RedWorker *worker, int *ring_is_empty)
if (!red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
red_drawable, ext_cmd.cmd.data, ext_cmd.flags)) {
- display_channel_process_draw(worker->display_channel, red_drawable,
- worker->process_display_generation);
+ if (!red_drawable_joinable(red_drawable)) {
+ // not joinable, process all
+ if (worker->joinable_drawable) {
+ display_channel_process_draw(worker->display_channel, worker->joinable_drawable,
+ worker->process_display_generation);
+ red_drawable_unref(worker->joinable_drawable);
+ worker->joinable_drawable = NULL;
+ }
+ display_channel_process_draw(worker->display_channel, red_drawable,
+ worker->process_display_generation);
+ red_drawable_unref(red_drawable);
+ break;
+ } else if (worker->joinable_drawable == NULL) {
+ // try to join with next one
+ } else if (red_drawable_can_join(worker->joinable_drawable, red_drawable)) {
+ red_drawable = red_drawable_join(worker->joinable_drawable, red_drawable);
+ } else {
+ // they can't be joined
+ display_channel_process_draw(worker->display_channel, worker->joinable_drawable,
+ worker->process_display_generation);
+ red_drawable_unref(worker->joinable_drawable);
+ }
+ // try to join with next one
+ worker->joinable_drawable = red_drawable;
+ break;
}
// release the red_drawable
red_drawable_unref(red_drawable);
--
2.9.3
More information about the Spice-devel
mailing list