On Mon, Jun 25, 2018 at 10:50:27AM +0100, Frediano Ziglio wrote:
> 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@xxxxxxxxxx>
> ---
> server/red-parse-qxl.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> Changes since v1:
> - handle all QXL_DRAW_COPY paths parsing the command;
> - more long explanation.
>
> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index 4a45c0f7..ca9b27a3 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -677,8 +677,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;
Looks to me, ideally we could warn once when this is unexpectedly set,
but we are missing helpers for that at the moment.
Looks good otherwise,
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Christophe
> +
> + 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;
> @@ -1049,9 +1056,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;
> @@ -1128,9 +1135,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;
> --
> 2.17.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel