[Spice-commits] server/red-parse-qxl.c

[GitLab Mirror]

 server/red-parse-qxl.c |   17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

New commits:
commit ca4984570f425e87e92abe5f62f9687bb55c1e14
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Jun 22 06:59:26 2018 +0100

    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 at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 2e1f2b48..dd840be1 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -671,8 +671,15 @@ static void red_put_opaque(SpiceOpaque *red)
 }
 
 static bool 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 false;
@@ -1038,9 +1045,9 @@ static bool red_get_native_drawable(RedMemSlotInfo *slots, int group_id,
                               &red->u.blackness, &qxl->u.blackness, flags);
         break;
     case QXL_DRAW_BLEND:
-        return red_get_blend_ptr(slots, group_id, &red->u.blend, &qxl->u.blend, flags);
+        return red_get_blend_ptr(slots, group_id, red, &qxl->u.blend, flags);
     case QXL_DRAW_COPY:
-        return red_get_copy_ptr(slots, group_id, &red->u.copy, &qxl->u.copy, flags);
+        return red_get_copy_ptr(slots, group_id, red, &qxl->u.copy, flags);
     case QXL_COPY_BITS:
         red_get_point_ptr(&red->u.copy_bits.src_pos, &qxl->u.copy_bits.src_pos);
         break;
@@ -1116,9 +1123,9 @@ static bool red_get_compat_drawable(RedMemSlotInfo *slots, int group_id,
                               &red->u.blackness, &qxl->u.blackness, flags);
         break;
     case QXL_DRAW_BLEND:
-        return red_get_blend_ptr(slots, group_id, &red->u.blend, &qxl->u.blend, flags);
+        return red_get_blend_ptr(slots, group_id, red, &qxl->u.blend, flags);
     case QXL_DRAW_COPY:
-        return red_get_copy_ptr(slots, group_id, &red->u.copy, &qxl->u.copy, flags);
+        return red_get_copy_ptr(slots, group_id, red, &qxl->u.copy, flags);
     case QXL_COPY_BITS:
         red_get_point_ptr(&red->u.copy_bits.src_pos, &qxl->u.copy_bits.src_pos);
         red->surface_deps[0] = 0;