From ca4984570f425e87e92abe5f62f9687bb55c1e14 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@redhat.com>
Date: Fri, 22 Jun 2018 06:59:26 +0100
Subject: [PATCH] red-parse-qxl: Avoid invalid flag usage

self_bitmap flag is used for some complex drawing not possible
by QXL_DRAW_COPY commands. Having this flag set causes
spice-server do draw part of the screen, copy that part on new
allocated image and reduce network optimisations with no visual
changes.
Some drivers (like Windows 10 DOD) set this flag by mistake for
this command so reset it.

More details follow.

The self_bitmap flag is used for some drawing command requiring to mix
the frame buffer with some other image. For this specific
QXL_DRAW_COPY command self_bitmap is used by spice-server code during
cachine/sending (the reason for the cache is to cache images sent to
client so the relationship between the two parts of the code).
However the self_bitmap_image (an image created in spice-server if
this flags is set) is used only if src_bitmap of SpiceCopy structure
(the structure used to store the QXL_DRAW_COPY command inside
spice-server) is NULL. But in red_get_copy_ptr (red-parse-qxl.c, the
function that parse the QXL_DRAW_COPY command form the QXL device)
not having a src_bitmap is considered an error so the
self_bitmap_image won't be used.

Why this flag affects network performance?
When spice-server see this flag it update the frame buffer according
to the pending commands (commands to be sent or still to be drawn on
frame buffer). spice-server maintain a tree of commands used to reduce
rendering and command to send. More or less if a command is covering
other commands (for instance filling the entire screen with a single
color) the pending commands can be removed from the queue and not sent
to the client. However when an update of the frame buffer is requested
spice-server update the frame buffer removing the commands from the
tree but not from the client queue.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
[BR: BSC#1109044]
(cherry picked from commit ca4984570f425e87e92abe5f62f9687bb55c1e14)
Signed-off-by: Bruce Rogers <brogers@suse.com>
---
 server/red_parse_qxl.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
index 8a35b29e..719fcf8f 100644
--- a/server/red_parse_qxl.c
+++ b/server/red_parse_qxl.c
@@ -655,8 +655,15 @@ static void red_put_opaque(SpiceOpaque *red)
 }
 
 static int red_get_copy_ptr(RedMemSlotInfo *slots, int group_id,
-                            SpiceCopy *red, QXLCopy *qxl, uint32_t flags)
+                             RedDrawable *red_drawable, QXLCopy *qxl, uint32_t flags)
 {
+    /* there's no sense to have this true, this will just waste CPU and reduce optimizations
+     * for this command. Due to some bugs however some driver set self_bitmap field for this
+     * command so reset it. */
+    red_drawable->self_bitmap = false;
+
+    SpiceCopy *red = &red_drawable->u.copy;
+
     red->src_bitmap      = red_get_image(slots, group_id, qxl->src_bitmap, flags, FALSE);
     if (!red->src_bitmap) {
         return 1;
@@ -1035,10 +1042,10 @@ static int red_get_native_drawable(RedMemSlotInfo *slots, int group_id,
                               &red->u.blackness, &qxl->u.blackness, flags);
         break;
     case QXL_DRAW_BLEND:
-        error = red_get_blend_ptr(slots, group_id, &red->u.blend, &qxl->u.blend, flags);
+        error = red_get_blend_ptr(slots, group_id, red, &qxl->u.blend, flags);
         break;
     case QXL_DRAW_COPY:
-        error = red_get_copy_ptr(slots, group_id, &red->u.copy, &qxl->u.copy, flags);
+        error = red_get_copy_ptr(slots, group_id, red, &qxl->u.copy, flags);
         break;
     case QXL_COPY_BITS:
         red_get_point_ptr(&red->u.copy_bits.src_pos, &qxl->u.copy_bits.src_pos);
@@ -1117,10 +1124,10 @@ static int red_get_compat_drawable(RedMemSlotInfo *slots, int group_id,
                               &red->u.blackness, &qxl->u.blackness, flags);
         break;
     case QXL_DRAW_BLEND:
-        error = red_get_blend_ptr(slots, group_id, &red->u.blend, &qxl->u.blend, flags);
+        error = red_get_blend_ptr(slots, group_id, red, &qxl->u.blend, flags);
         break;
     case QXL_DRAW_COPY:
-        error = red_get_copy_ptr(slots, group_id, &red->u.copy, &qxl->u.copy, flags);
+        error = red_get_copy_ptr(slots, group_id, red, &qxl->u.copy, flags);
         break;
     case QXL_COPY_BITS:
         red_get_point_ptr(&red->u.copy_bits.src_pos, &qxl->u.copy_bits.src_pos);
-- 
2.19.0

