[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 <address@hidden>

---

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_;