From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Lhlmu-0001VK-9q for qemu-devel@nongnu.org; Thu, 12 Mar 2009 10:21:32 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Lhlmt-0001V0-DK for qemu-devel@nongnu.org; Thu, 12 Mar 2009 10:21:31 -0400 Received: from [199.232.76.173] (port=57336 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Lhlmt-0001Ux-A6 for qemu-devel@nongnu.org; Thu, 12 Mar 2009 10:21:31 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:32951) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Lhlms-0005Oa-Vd for qemu-devel@nongnu.org; Thu, 12 Mar 2009 10:21:31 -0400 Received: from [10.80.225.184] ([10.80.225.184]) by smtp01.ad.xensource.com (8.13.1/8.13.1) with ESMTP id n2CELR3d031931 for ; Thu, 12 Mar 2009 07:21:28 -0700 Message-ID: <49B9193A.7090001@eu.citrix.com> Date: Thu, 12 Mar 2009 14:16:26 +0000 From: Stefano Stabellini MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH] vnc: shared buffer: skip some optimizations. References: <49B7CE21.4030104@redhat.com> In-Reply-To: <49B7CE21.4030104@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "qemu-devel@nongnu.org" Gerd Hoffmann wrote: >diff --git a/vnc.c b/vnc.c >index 81c842a..66f8946 100644 >--- a/vnc.c >+++ b/vnc.c >@@ -649,7 +649,8 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int > VncDisplay *vd = ds->opaque; > VncState *vs = vd->clients; > while (vs != NULL) { >- if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) >+ if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT) && >+ !is_buffer_shared(ds->surface)) > vnc_copy(vs, src_x, src_y, dst_x, dst_y, w, h); > else /* TODO */ > vnc_update(vs, dst_x, dst_y, w, h); > I don't think this is needed (on qemu and qemu-xen, I don't know about kvm): vnc_copy does not do any actual copy, only sends a copyrect update to the vnc client. Why should we prevent this when the buffer is shared? >@@ -688,36 +689,51 @@ static void vnc_update_client(void *opaque) > > vnc_set_bits(width_mask, (ds_get_width(vs->ds) / 16), VNC_DIRTY_WORDS); > >- /* Walk through the dirty map and eliminate tiles that >- really aren't dirty */ >- row = ds_get_data(vs->ds); >- old_row = vs->old_data; >- >- for (y = 0; y < ds_get_height(vs->ds); y++) { >- if (vnc_and_bits(vs->dirty_row[y], width_mask, VNC_DIRTY_WORDS)) { >- int x; >- uint8_t *ptr; >- char *old_ptr; >- >- ptr = row; >- old_ptr = (char*)old_row; >- >- for (x = 0; x < ds_get_width(vs->ds); x += 16) { >- if (memcmp(old_ptr, ptr, 16 * ds_get_bytes_per_pixel(vs->ds)) == 0) { >- vnc_clear_bit(vs->dirty_row[y], (x / 16)); >- } else { >- has_dirty = 1; >- memcpy(old_ptr, ptr, 16 * ds_get_bytes_per_pixel(vs->ds)); >- } >- >- ptr += 16 * ds_get_bytes_per_pixel(vs->ds); >- old_ptr += 16 * ds_get_bytes_per_pixel(vs->ds); >- } >- } > This loop filters the dirty_row bitmap using a memcmp against the framebuffer, that could be more up to date than the bitmap itself. In any case we are getting another vnc_dpy_update call next time with the correct updated area. So I don't think we risk losing updates here, the worst that could happen is sending together portions of the screen more updated than others to the client. I would keep this loop even with the buffer shared. Gerd, am I missing something?