qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 3 of 4] [UPDATE ]single vnc server surface
@ 2009-07-30 13:18 Stefano Stabellini
  2009-07-31  8:04 ` Gerd Hoffmann
  0 siblings, 1 reply; 3+ messages in thread
From: Stefano Stabellini @ 2009-07-30 13:18 UTC (permalink / raw)
  To: qemu-devel

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.

This patch has been updated to handle copyrect correctly.

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

---

diff --git a/vnc.c b/vnc.c
index a57516c..044bae8 100644
--- a/vnc.c
+++ b/vnc.c
@@ -215,13 +215,17 @@ 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);
 static void vnc_remove_timer(VncDisplay *vd);
 
 static void vnc_colordepth(VncState *vs);
+static void framebuffer_update_request(VncState *vs, int incremental,
+                                       int x_position, int y_position,
+                                       int w, int h);
+static void vnc_refresh(void *opaque);
 
 static inline void vnc_set_bit(uint32_t *d, int k)
 {
@@ -264,10 +268,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 +293,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 +337,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 +389,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 +433,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 +443,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 +451,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 +468,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 +518,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) {
@@ -656,10 +650,6 @@ static void send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
 
 static void vnc_copy(VncState *vs, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
 {
-    uint8_t *src_row;
-    uint8_t *dst_row;
-    int y,pitch,depth;
-
     /* send bitblit op to the vnc client */
     vnc_write_u8(vs, 0);  /* msg id */
     vnc_write_u8(vs, 0);
@@ -668,12 +658,23 @@ static void vnc_copy(VncState *vs, int src_x, int src_y, int dst_x, int dst_y, i
     vnc_write_u16(vs, src_x);
     vnc_write_u16(vs, src_y);
     vnc_flush(vs);
+}
+
+static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
+{
+    VncDisplay *vd = ds->opaque;
+    VncState *vs;
+    uint8_t *src_row;
+    uint8_t *dst_row;
+    int y,pitch,depth;
+
+    vnc_refresh(vd);
 
     /* do bitblit op on the local surface too */
-    pitch = ds_get_linesize(vs->ds);
-    depth = ds_get_bytes_per_pixel(vs->ds);
-    src_row = vs->server.ds->data + pitch * src_y + depth * src_x;
-    dst_row = vs->server.ds->data + pitch * dst_y + depth * dst_x;
+    pitch = ds_get_linesize(vd->ds);
+    depth = ds_get_bytes_per_pixel(vd->ds);
+    src_row = vd->server->data + pitch * src_y + depth * src_x;
+    dst_row = vd->server->data + pitch * dst_y + depth * dst_x;
     if (dst_y > src_y) {
         /* copy backwards */
         src_row += pitch * (h-1);
@@ -685,96 +686,44 @@ static void vnc_copy(VncState *vs, int src_x, int src_y, int dst_x, int dst_y, i
         src_row += pitch;
         dst_row += pitch;
     }
-}
-
-static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
-{
-    VncDisplay *vd = ds->opaque;
-    VncState *vs, *vn;
-
-    for (vs = vd->clients; vs != NULL; vs = vn) {
-        vn = vs->next;
-        if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
-            vs->force_update = 1;
-            vnc_update_client(vs);
-            /* vs might be free()ed here */
-        }
-    }
 
     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);
+        else
+            framebuffer_update_request(vs, 0, 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;
 
@@ -790,18 +739,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++;
                     }
@@ -809,7 +758,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++;
             }
@@ -926,9 +875,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);
 }
@@ -1507,9 +1453,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);
         }
     }
@@ -1638,7 +1582,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;
@@ -2070,12 +2014,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;
     }
 
@@ -2086,7 +2075,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);
     }
 }
 
@@ -2119,17 +2108,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_;

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH 3 of 4] [UPDATE ]single vnc server surface
  2009-07-30 13:18 [Qemu-devel] [PATCH 3 of 4] [UPDATE ]single vnc server surface Stefano Stabellini
@ 2009-07-31  8:04 ` Gerd Hoffmann
  2009-07-31 16:51   ` Stefano Stabellini
  0 siblings, 1 reply; 3+ messages in thread
From: Gerd Hoffmann @ 2009-07-31  8:04 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: qemu-devel

On 07/30/09 15:18, 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.
>
> This patch has been updated to handle copyrect correctly.

Well.  Sort of.  At least it has no screen corruption.  The patch kills 
a number of bandwith saving optimizations though.

Number one (the big one):  vnc clients without copyrect support get a 
*huge* penalty.  Each vnc_copy call now sends a screen refresh to *all* 
clients, not only the ones with copyrect support.  And they get the 
*whole* destination rectangle, not only the screen areas which did 
actually change.  Given that vnc_copy can happen much more frequently 
than the refresh interval I think this increases the bandwith used *alot*.

Number two: In case we skip frames because we have data buffered (i.e. 
kernel output pipe is full) we might send more updates than we have to. 
  With your patch the dirty bits of all screen updates are combined 
together, including the screen updates skipped.  So screen areas which 
changed in the skipped frame, then changed back for the frame we 
actually sent are updated nevertheless.  This isn't the case without the 
patch.

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH 3 of 4] [UPDATE ]single vnc server surface
  2009-07-31  8:04 ` Gerd Hoffmann
@ 2009-07-31 16:51   ` Stefano Stabellini
  0 siblings, 0 replies; 3+ messages in thread
From: Stefano Stabellini @ 2009-07-31 16:51 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel@nongnu.org, Stefano Stabellini

On Fri, 31 Jul 2009, Gerd Hoffmann wrote:
> On 07/30/09 15:18, 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.
> >
> > This patch has been updated to handle copyrect correctly.
> 
> Well.  Sort of.  At least it has no screen corruption.  The patch kills 
> a number of bandwith saving optimizations though.
> 
> Number one (the big one):  vnc clients without copyrect support get a 
> *huge* penalty.  Each vnc_copy call now sends a screen refresh to *all* 
> clients, not only the ones with copyrect support.  And they get the 
> *whole* destination rectangle, not only the screen areas which did 
> actually change.  Given that vnc_copy can happen much more frequently 
> than the refresh interval I think this increases the bandwith used *alot*.

I admit I didn't pay too much attention to copyrect performances because
everyone that is interested in performances should disable hw
acceleration in the VM for cirrus: software emulated hw acceleration is
between 5 and 10 times slower than plain old vesa.

That said, I am all for improving everything that can be improved, so I
made further enhancements to this patch, addressing these problems: now
copyrect should have the same performances as before (thank you for the
very detailed description of the issues present).

I am going to send an update of this patch and the following one that
needs a rebase.


> Number two: In case we skip frames because we have data buffered (i.e. 
> kernel output pipe is full) we might send more updates than we have to. 
>   With your patch the dirty bits of all screen updates are combined 
> together, including the screen updates skipped.  So screen areas which 
> changed in the skipped frame, then changed back for the frame we 
> actually sent are updated nevertheless.  This isn't the case without the 
> patch.
> 

I don't think I can do much about this.
In any case this issue shouldn't affect performances significantly: the
benefits of the patch are still greater than the disadvantages.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-07-31 16:50 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-30 13:18 [Qemu-devel] [PATCH 3 of 4] [UPDATE ]single vnc server surface Stefano Stabellini
2009-07-31  8:04 ` Gerd Hoffmann
2009-07-31 16:51   ` Stefano Stabellini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).