[Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Stefano Stabellini]

This patch removes the server surface from VncState and adds a single
server surface to VncDisplay for all the possible clients connected.
Each client maintains a different dirty bitmap in VncState.
The guest surface is moved to VncDisplay as well because we don't need
to track guest updates in more than one place.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

diff --git a/vnc.c b/vnc.c
index c213626..fa8547f 100644
--- a/vnc.c
+++ b/vnc.c
@@ -215,7 +215,7 @@ static inline uint32_t vnc_has_feature(VncState *vs, int feature) {
    3) resolutions > 1024
 */
 
-static void vnc_update_client(VncState *vs);
+static void vnc_update_client(VncState *vs, int has_dirty);
 static void vnc_disconnect_start(VncState *vs);
 static void vnc_disconnect_finish(VncState *vs);
 static void vnc_init_timer(VncDisplay *vd);
@@ -264,10 +264,11 @@ static inline int vnc_and_bits(const uint32_t *d1, const uint32_t *d2,
     return 0;
 }
 
-static void vnc_update(VncState *vs, int x, int y, int w, int h)
+static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h)
 {
-    struct VncSurface *s = &vs->guest;
     int i;
+    VncDisplay *vd = ds->opaque;
+    struct VncSurface *s = &vd->guest;
 
     h += y;
 
@@ -288,16 +289,6 @@ static void vnc_update(VncState *vs, int x, int y, int w, int h)
             vnc_set_bit(s->dirty[y], (x + i) / 16);
 }
 
-static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h)
-{
-    VncDisplay *vd = ds->opaque;
-    VncState *vs = vd->clients;
-    while (vs != NULL) {
-        vnc_update(vs, x, y, w, h);
-        vs = vs->next;
-    }
-}
-
 static void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
                                    int32_t encoding)
 {
@@ -342,49 +333,44 @@ void buffer_append(Buffer *buffer, const void *data, size_t len)
     buffer->offset += len;
 }
 
-static void vnc_resize(VncState *vs)
+static void vnc_dpy_resize(DisplayState *ds)
 {
-    DisplayState *ds = vs->ds;
     int size_changed;
+    VncDisplay *vd = ds->opaque;
+    VncState *vs = vd->clients;
+
+    /* server surface */
+    if (!vd->server)
+        vd->server = qemu_mallocz(sizeof(*vd->server));
+    if (vd->server->data)
+        qemu_free(vd->server->data);
+    *(vd->server) = *(ds->surface);
+    vd->server->data = qemu_mallocz(vd->server->linesize *
+                                    vd->server->height);
 
     /* guest surface */
-    if (!vs->guest.ds)
-        vs->guest.ds = qemu_mallocz(sizeof(*vs->guest.ds));
-    if (ds_get_bytes_per_pixel(ds) != vs->guest.ds->pf.bytes_per_pixel)
+    if (!vd->guest.ds)
+        vd->guest.ds = qemu_mallocz(sizeof(*vd->guest.ds));
+    if (ds_get_bytes_per_pixel(ds) != vd->guest.ds->pf.bytes_per_pixel)
         console_color_init(ds);
-    vnc_colordepth(vs);
-    size_changed = ds_get_width(ds) != vs->guest.ds->width ||
-                   ds_get_height(ds) != vs->guest.ds->height;
-    *(vs->guest.ds) = *(ds->surface);
-    if (size_changed) {
-        if (vs->csock != -1 && vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
-            vnc_write_u8(vs, 0);  /* msg id */
-            vnc_write_u8(vs, 0);
-            vnc_write_u16(vs, 1); /* number of rects */
-            vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
-                                   VNC_ENCODING_DESKTOPRESIZE);
-            vnc_flush(vs);
-        }
-    }
-    memset(vs->guest.dirty, 0xFF, sizeof(vs->guest.dirty));
+    size_changed = ds_get_width(ds) != vd->guest.ds->width ||
+                   ds_get_height(ds) != vd->guest.ds->height;
+    *(vd->guest.ds) = *(ds->surface);
+    memset(vd->guest.dirty, 0xFF, sizeof(vd->guest.dirty));
 
-    /* server surface */
-    if (!vs->server.ds)
-        vs->server.ds = qemu_mallocz(sizeof(*vs->server.ds));
-    if (vs->server.ds->data)
-        qemu_free(vs->server.ds->data);
-    *(vs->server.ds) = *(ds->surface);
-    vs->server.ds->data = qemu_mallocz(vs->server.ds->linesize *
-                                       vs->server.ds->height);
-    memset(vs->server.dirty, 0xFF, sizeof(vs->guest.dirty));
-}
-
-static void vnc_dpy_resize(DisplayState *ds)
-{
-    VncDisplay *vd = ds->opaque;
-    VncState *vs = vd->clients;
     while (vs != NULL) {
-        vnc_resize(vs);
+        vnc_colordepth(vs);
+        if (size_changed) {
+            if (vs->csock != -1 && vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
+                vnc_write_u8(vs, 0);  /* msg id */
+                vnc_write_u8(vs, 0);
+                vnc_write_u16(vs, 1); /* number of rects */
+                vnc_framebuffer_update(vs, 0, 0, ds_get_width(ds), ds_get_height(ds),
+                        VNC_ENCODING_DESKTOPRESIZE);
+                vnc_flush(vs);
+            }
+        }
+        memset(vs->dirty, 0xFF, sizeof(vs->dirty));
         vs = vs->next;
     }
 }
@@ -399,13 +385,14 @@ static void vnc_write_pixels_copy(VncState *vs, void *pixels, int size)
 static void vnc_convert_pixel(VncState *vs, uint8_t *buf, uint32_t v)
 {
     uint8_t r, g, b;
-
-    r = ((((v & vs->server.ds->pf.rmask) >> vs->server.ds->pf.rshift) << vs->clientds.pf.rbits) >>
-        vs->server.ds->pf.rbits);
-    g = ((((v & vs->server.ds->pf.gmask) >> vs->server.ds->pf.gshift) << vs->clientds.pf.gbits) >>
-        vs->server.ds->pf.gbits);
-    b = ((((v & vs->server.ds->pf.bmask) >> vs->server.ds->pf.bshift) << vs->clientds.pf.bbits) >>
-        vs->server.ds->pf.bbits);
+    VncDisplay *vd = vs->vd;
+
+    r = ((((v & vd->server->pf.rmask) >> vd->server->pf.rshift) << vs->clientds.pf.rbits) >>
+        vd->server->pf.rbits);
+    g = ((((v & vd->server->pf.gmask) >> vd->server->pf.gshift) << vs->clientds.pf.gbits) >>
+        vd->server->pf.gbits);
+    b = ((((v & vd->server->pf.bmask) >> vd->server->pf.bshift) << vs->clientds.pf.bbits) >>
+        vd->server->pf.bbits);
     v = (r << vs->clientds.pf.rshift) |
         (g << vs->clientds.pf.gshift) |
         (b << vs->clientds.pf.bshift);
@@ -442,8 +429,9 @@ static void vnc_convert_pixel(VncState *vs, uint8_t *buf, uint32_t v)
 static void vnc_write_pixels_generic(VncState *vs, void *pixels1, int size)
 {
     uint8_t buf[4];
+    VncDisplay *vd = vs->vd;
 
-    if (vs->server.ds->pf.bytes_per_pixel == 4) {
+    if (vd->server->pf.bytes_per_pixel == 4) {
         uint32_t *pixels = pixels1;
         int n, i;
         n = size >> 2;
@@ -451,7 +439,7 @@ static void vnc_write_pixels_generic(VncState *vs, void *pixels1, int size)
             vnc_convert_pixel(vs, buf, pixels[i]);
             vnc_write(vs, buf, vs->clientds.pf.bytes_per_pixel);
         }
-    } else if (vs->server.ds->pf.bytes_per_pixel == 2) {
+    } else if (vd->server->pf.bytes_per_pixel == 2) {
         uint16_t *pixels = pixels1;
         int n, i;
         n = size >> 1;
@@ -459,7 +447,7 @@ static void vnc_write_pixels_generic(VncState *vs, void *pixels1, int size)
             vnc_convert_pixel(vs, buf, pixels[i]);
             vnc_write(vs, buf, vs->clientds.pf.bytes_per_pixel);
         }
-    } else if (vs->server.ds->pf.bytes_per_pixel == 1) {
+    } else if (vd->server->pf.bytes_per_pixel == 1) {
         uint8_t *pixels = pixels1;
         int n, i;
         n = size;
@@ -476,8 +464,9 @@ static void send_framebuffer_update_raw(VncState *vs, int x, int y, int w, int h
 {
     int i;
     uint8_t *row;
+    VncDisplay *vd = vs->vd;
 
-    row = vs->server.ds->data + y * ds_get_linesize(vs->ds) + x * ds_get_bytes_per_pixel(vs->ds);
+    row = vd->server->data + y * ds_get_linesize(vs->ds) + x * ds_get_bytes_per_pixel(vs->ds);
     for (i = 0; i < h; i++) {
         vs->write_pixels(vs, row, w * ds_get_bytes_per_pixel(vs->ds));
         row += ds_get_linesize(vs->ds);
@@ -525,9 +514,10 @@ static void send_framebuffer_update_hextile(VncState *vs, int x, int y, int w, i
     int i, j;
     int has_fg, has_bg;
     uint8_t *last_fg, *last_bg;
+    VncDisplay *vd = vs->vd;
 
-    last_fg = (uint8_t *) qemu_malloc(vs->server.ds->pf.bytes_per_pixel);
-    last_bg = (uint8_t *) qemu_malloc(vs->server.ds->pf.bytes_per_pixel);
+    last_fg = (uint8_t *) qemu_malloc(vd->server->pf.bytes_per_pixel);
+    last_bg = (uint8_t *) qemu_malloc(vd->server->pf.bytes_per_pixel);
     has_fg = has_bg = 0;
     for (j = y; j < (y + h); j += 16) {
         for (i = x; i < (x + w); i += 16) {
@@ -674,7 +664,7 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int
         vn = vs->next;
         if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
             vs->force_update = 1;
-            vnc_update_client(vs);
+            vnc_update_client(vs, 1);
             /* vs might be free()ed here */
         }
     }
@@ -682,77 +672,39 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int
     for (vs = vd->clients; vs != NULL; vs = vs->next) {
         if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT))
             vnc_copy(vs, src_x, src_y, dst_x, dst_y, w, h);
-        else /* TODO */
-            vnc_update(vs, dst_x, dst_y, w, h);
     }
+    vnc_dpy_update(ds, dst_x, dst_y, w, h);
 }
 
-static int find_and_clear_dirty_height(struct VncSurface *s,
+static int find_and_clear_dirty_height(struct VncState *vs,
                                        int y, int last_x, int x)
 {
     int h;
+    VncDisplay *vd = vs->vd;
 
-    for (h = 1; h < (s->ds->height - y); h++) {
+    for (h = 1; h < (vd->server->height - y); h++) {
         int tmp_x;
-        if (!vnc_get_bit(s->dirty[y + h], last_x))
+        if (!vnc_get_bit(vs->dirty[y + h], last_x))
             break;
         for (tmp_x = last_x; tmp_x < x; tmp_x++)
-            vnc_clear_bit(s->dirty[y + h], tmp_x);
+            vnc_clear_bit(vs->dirty[y + h], tmp_x);
     }
 
     return h;
 }
 
-static void vnc_update_client(VncState *vs)
+static void vnc_update_client(VncState *vs, int has_dirty)
 {
     if (vs->need_update && vs->csock != -1) {
+        VncDisplay *vd = vs->vd;
         int y;
-        uint8_t *guest_row;
-        uint8_t *server_row;
-        int cmp_bytes;
-        uint32_t width_mask[VNC_DIRTY_WORDS];
         int n_rectangles;
         int saved_offset;
-        int has_dirty = 0;
 
         if (vs->output.offset && !vs->audio_cap && !vs->force_update)
             /* kernel send buffers are full -> drop frames to throttle */
             return;
 
-        /*
-         * Walk through the guest dirty map.
-         * Check and copy modified bits from guest to server surface.
-         * Update server dirty map.
-         */
-        vnc_set_bits(width_mask, (ds_get_width(vs->ds) / 16), VNC_DIRTY_WORDS);
-        cmp_bytes = 16 * ds_get_bytes_per_pixel(vs->ds);
-        guest_row  = vs->guest.ds->data;
-        server_row = vs->server.ds->data;
-        for (y = 0; y < vs->guest.ds->height; y++) {
-            if (vnc_and_bits(vs->guest.dirty[y], width_mask, VNC_DIRTY_WORDS)) {
-                int x;
-                uint8_t *guest_ptr;
-                uint8_t *server_ptr;
-
-                guest_ptr  = guest_row;
-                server_ptr = server_row;
-
-                for (x = 0; x < vs->guest.ds->width;
-                     x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
-                    if (!vnc_get_bit(vs->guest.dirty[y], (x / 16)))
-                        continue;
-                    vnc_clear_bit(vs->guest.dirty[y], (x / 16));
-                    if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0)
-                        continue;
-                    memcpy(server_ptr, guest_ptr, cmp_bytes);
-                    vnc_set_bit(vs->server.dirty[y], (x / 16));
-                    has_dirty++;
-                }
-            }
-            guest_row  += ds_get_linesize(vs->ds);
-            server_row += ds_get_linesize(vs->ds);
-        }
-
         if (!has_dirty && !vs->audio_cap && !vs->force_update)
             return;
 
@@ -768,18 +720,18 @@ static void vnc_update_client(VncState *vs)
         saved_offset = vs->output.offset;
         vnc_write_u16(vs, 0);
 
-        for (y = 0; y < vs->server.ds->height; y++) {
+        for (y = 0; y < vd->server->height; y++) {
             int x;
             int last_x = -1;
-            for (x = 0; x < vs->server.ds->width / 16; x++) {
-                if (vnc_get_bit(vs->server.dirty[y], x)) {
+            for (x = 0; x < vd->server->width / 16; x++) {
+                if (vnc_get_bit(vs->dirty[y], x)) {
                     if (last_x == -1) {
                         last_x = x;
                     }
-                    vnc_clear_bit(vs->server.dirty[y], x);
+                    vnc_clear_bit(vs->dirty[y], x);
                 } else {
                     if (last_x != -1) {
-                        int h = find_and_clear_dirty_height(&vs->server, y, last_x, x);
+                        int h = find_and_clear_dirty_height(vs, y, last_x, x);
                         send_framebuffer_update(vs, last_x * 16, y, (x - last_x) * 16, h);
                         n_rectangles++;
                     }
@@ -787,7 +739,7 @@ static void vnc_update_client(VncState *vs)
                 }
             }
             if (last_x != -1) {
-                int h = find_and_clear_dirty_height(&vs->server, y, last_x, x);
+                int h = find_and_clear_dirty_height(vs, y, last_x, x);
                 send_framebuffer_update(vs, last_x * 16, y, (x - last_x) * 16, h);
                 n_rectangles++;
             }
@@ -904,9 +856,6 @@ static void vnc_disconnect_finish(VncState *vs)
     if (!vs->vd->clients)
         dcl->idle = 1;
 
-    qemu_free(vs->server.ds->data);
-    qemu_free(vs->server.ds);
-    qemu_free(vs->guest.ds);
     qemu_free(vs);
     vnc_remove_timer(vs->vd);
 }
@@ -1485,9 +1434,7 @@ static void framebuffer_update_request(VncState *vs, int incremental,
     if (!incremental) {
         vs->force_update = 1;
         for (i = 0; i < h; i++) {
-            vnc_set_bits(vs->guest.dirty[y_position + i],
-                         (ds_get_width(vs->ds) / 16), VNC_DIRTY_WORDS);
-            vnc_set_bits(vs->server.dirty[y_position + i],
+            vnc_set_bits(vs->dirty[y_position + i],
                          (ds_get_width(vs->ds) / 16), VNC_DIRTY_WORDS);
         }
     }
@@ -1616,7 +1563,7 @@ static void set_pixel_format(VncState *vs,
         return;
     }
 
-    vs->clientds = *(vs->guest.ds);
+    vs->clientds = *(vs->vd->guest.ds);
     vs->clientds.pf.rmax = red_max;
     count_bits(vs->clientds.pf.rbits, red_max);
     vs->clientds.pf.rshift = red_shift;
@@ -2048,12 +1995,57 @@ static int protocol_version(VncState *vs, uint8_t *version, size_t len)
 static void vnc_refresh(void *opaque)
 {
     VncDisplay *vd = opaque;
-    VncState *vs = vd->clients;
+    VncState *vs = NULL;
+    int y;
+    uint8_t *guest_row;
+    uint8_t *server_row;
+    int cmp_bytes;
+    uint32_t width_mask[VNC_DIRTY_WORDS];
+    int has_dirty = 0;
 
     vga_hw_update();
 
+    /*
+     * Walk through the guest dirty map.
+     * Check and copy modified bits from guest to server surface.
+     * Update server dirty map.
+     */
+    vnc_set_bits(width_mask, (ds_get_width(vd->ds) / 16), VNC_DIRTY_WORDS);
+    cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds);
+    guest_row  = vd->guest.ds->data;
+    server_row = vd->server->data;
+    for (y = 0; y < vd->guest.ds->height; y++) {
+        if (vnc_and_bits(vd->guest.dirty[y], width_mask, VNC_DIRTY_WORDS)) {
+            int x;
+            uint8_t *guest_ptr;
+            uint8_t *server_ptr;
+
+            guest_ptr  = guest_row;
+            server_ptr = server_row;
+
+            for (x = 0; x < vd->guest.ds->width;
+                    x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
+                if (!vnc_get_bit(vd->guest.dirty[y], (x / 16)))
+                    continue;
+                vnc_clear_bit(vd->guest.dirty[y], (x / 16));
+                if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0)
+                    continue;
+                memcpy(server_ptr, guest_ptr, cmp_bytes);
+                vs = vd->clients;
+                while (vs != NULL) {
+                    vnc_set_bit(vs->dirty[y], (x / 16));
+                    vs = vs->next;
+                }
+                has_dirty++;
+            }
+        }
+        guest_row  += ds_get_linesize(vd->ds);
+        server_row += ds_get_linesize(vd->ds);
+    }
+
+    vs = vd->clients;
     while (vs != NULL) {
-        vnc_update_client(vs);
+        vnc_update_client(vs, has_dirty);
         vs = vs->next;
     }
 
@@ -2064,7 +2056,7 @@ static void vnc_init_timer(VncDisplay *vd)
 {
     if (vd->timer == NULL && vd->clients != NULL) {
         vd->timer = qemu_new_timer(rt_clock, vnc_refresh, vd);
-        qemu_mod_timer(vd->timer, qemu_get_clock(rt_clock) + VNC_REFRESH_INTERVAL);
+        vnc_refresh(vd);
     }
 }
 
@@ -2097,17 +2089,18 @@ static void vnc_connect(VncDisplay *vd, int csock)
     vs->as.fmt = AUD_FMT_S16;
     vs->as.endianness = 0;
 
-    vnc_resize(vs);
+    vs->next = vd->clients;
+    vd->clients = vs;
+
+    vga_hw_update();
+
     vnc_write(vs, "RFB 003.008\n", 12);
     vnc_flush(vs);
     vnc_read_when(vs, protocol_version, 12);
     reset_keys(vs);
 
-    vs->next = vd->clients;
-    vd->clients = vs;
-
     vnc_init_timer(vd);
-    vnc_update_client(vs);
+
     /* vs might be free()ed here */
 }
 
diff --git a/vnc.h b/vnc.h
index a35b378..5c903de 100644
--- a/vnc.h
+++ b/vnc.h
@@ -84,6 +84,11 @@ typedef struct VncDisplay VncDisplay;
 #include "vnc-auth-sasl.h"
 #endif
 
+struct VncSurface
+{
+    uint32_t dirty[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS];
+    DisplaySurface *ds;
+};
 
 struct VncDisplay
 {
@@ -93,6 +98,9 @@ struct VncDisplay
     VncState *clients;
     kbd_layout_t *kbd_layout;
 
+    struct VncSurface guest;   /* guest visible surface (aka ds->surface) */
+    DisplaySurface *server;  /* vnc server surface */
+
     char *display;
     char *password;
     int auth;
@@ -105,19 +113,12 @@ struct VncDisplay
 #endif
 };
 
-struct VncSurface
-{
-    uint32_t dirty[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS];
-    DisplaySurface *ds;
-};
-
 struct VncState
 {
     int csock;
 
     DisplayState *ds;
-    struct VncSurface guest;   /* guest visible surface (aka ds->surface) */
-    struct VncSurface server;  /* vnc server surface */
+    uint32_t dirty[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS];
 
     VncDisplay *vd;
     int need_update;
diff --git a/vnchextile.h b/vnchextile.h
index f5b6fcb..c96ede3 100644
--- a/vnchextile.h
+++ b/vnchextile.h
@@ -13,7 +13,8 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
                                              void *last_fg_,
                                              int *has_bg, int *has_fg)
 {
-    uint8_t *row = vs->server.ds->data + y * ds_get_linesize(vs->ds) + x * ds_get_bytes_per_pixel(vs->ds);
+    VncDisplay *vd = vs->vd;
+    uint8_t *row = vd->server->data + y * ds_get_linesize(vs->ds) + x * ds_get_bytes_per_pixel(vs->ds);
     pixel_t *irow = (pixel_t *)row;
     int j, i;
     pixel_t *last_bg = (pixel_t *)last_bg_;

Re: [Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Gerd Hoffmann]

On 07/29/09 14:09, Stefano Stabellini wrote:
> This patch removes the server surface from VncState and adds a single
> server surface to VncDisplay for all the possible clients connected.
> Each client maintains a different dirty bitmap in VncState.
> The guest surface is moved to VncDisplay as well because we don't need
> to track guest updates in more than one place.

I doubt you did any copyrect testing.  It is broken in master[1], and 
this patch breaks it even more.

cheers,
   Gerd

[1] fix sent to the list on monday.

Re: [Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Stefano Stabellini]

On Wed, 29 Jul 2009, Gerd Hoffmann wrote:
> On 07/29/09 14:09, Stefano Stabellini wrote:
> > This patch removes the server surface from VncState and adds a single
> > server surface to VncDisplay for all the possible clients connected.
> > Each client maintains a different dirty bitmap in VncState.
> > The guest surface is moved to VncDisplay as well because we don't need
> > to track guest updates in more than one place.
> 
> I doubt you did any copyrect testing.  It is broken in master[1], and 
> this patch breaks it even more.

No I didn't: I notice that your fix wasn't applied yet but can be easily
applied on top of this patch and I just decided to rely on that.
You just have to s/vs->server.ds/vd->server/g.

Re: [Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Gerd Hoffmann]

On 07/29/09 16:40, Stefano Stabellini wrote:
> On Wed, 29 Jul 2009, Gerd Hoffmann wrote:
>> I doubt you did any copyrect testing.  It is broken in master[1], and
>> this patch breaks it even more.
>
> No I didn't: I notice that your fix wasn't applied yet but can be easily
> applied on top of this patch.

No, it can't.  Well, it will apply.  It will also build with the small 
fixup.  But the code isn't correct then.

The complete workflow of your patch was appearently designed without 
copyrect in mind.  The whole point of copyrect is to avoid sending 
screen updates.  Your patch doesn't handle that at all.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Stefano Stabellini]

On Wed, 29 Jul 2009, Gerd Hoffmann wrote:
> On 07/29/09 16:40, Stefano Stabellini wrote:
> > On Wed, 29 Jul 2009, Gerd Hoffmann wrote:
> >> I doubt you did any copyrect testing.  It is broken in master[1], and
> >> this patch breaks it even more.
> >
> > No I didn't: I notice that your fix wasn't applied yet but can be easily
> > applied on top of this patch.
> 
> No, it can't.  Well, it will apply.  It will also build with the small 
> fixup.  But the code isn't correct then.
> 
> The complete workflow of your patch was appearently designed without 
> copyrect in mind.  The whole point of copyrect is to avoid sending 
> screen updates.  Your patch doesn't handle that at all.
> 

All right, all right, I'll resend #2 with your patch integrated in the
right way. and I'll test copyrect myself.

Re: [Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Gerd Hoffmann]

On 07/29/09 17:05, Stefano Stabellini wrote:
> All right, all right, I'll resend #2 with your patch integrated in the
> right way. and I'll test copyrect myself.

Please keep the bugfix as separate patch.  Best before your patch #2, so 
we'll have a known-good state before doing the #2 reorganization.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Stefano Stabellini]

On Wed, 29 Jul 2009, Gerd Hoffmann wrote:
> I doubt you did any copyrect testing.  It is broken in master[1], and 
> this patch breaks it even more.
> 

What kind of test are you performing to be sure copyrect is working?
Because I am having trouble reproducing screen corruptions...

Re: [Qemu-devel] [PATCH 2 of 3] a single vnc server surface

[Gerd Hoffmann]

On 07/29/09 18:03, Stefano Stabellini wrote:
>
> On Wed, 29 Jul 2009, Gerd Hoffmann wrote:
>> I doubt you did any copyrect testing.  It is broken in master[1], and
>> this patch breaks it even more.
>>
>
> What kind of test are you performing to be sure copyrect is working?
> Because I am having trouble reproducing screen corruptions...

Start guest with cirrus vga, start X11 within the guest, make sure it 
actually uses the cirrus driver, open some xterm, type commands which 
trigger heavy scrolling (dmesg does the trick for me).

HTH,
   Gerd