Commit 6baebed7698a37a0ac5168faf26023426b0ac940

Authored by aliguori
1 parent a667866b

vnc: cleanup surface handling, fix screen corruption bug. (Gerd Hoffmann)

This patch killes the old_data hack in the qemu server and replaces
it with a clean separation of the guest-visible display surface and
the vnc server display surface.  Both guest and server surface have
their own dirty bitmap for tracking screen updates.

Workflow is this:

(1) The guest writes to the guest surface.  With shared buffers being
    active the guest writes are directly visible to the vnc server code.
    Note that this may happen in parallel to the vnc server code running
    (today only in xenfb, once we have vcpu threads in qemu also for
    other display adapters).

(2) vnc_update() callback tags the specified area in the guest dirty
    map.

(3) vnc_update_client() will first walk through the guest dirty map.  It
    will compare guest and server surface for all regions tagged dirty
    and in case the screen content really did change the server surface
    and dirty map are updated.
    Note: old code used old_data in a simliar way, so this does *not*
    introduce an extra memcpy.

(4) Then vnc_update_cient() will send the updates to the vnc client
    using the server surface and dirty map.
    Note: old code used the guest-visible surface instead, causing
    screen corruption in case of guest screen updates running in
    parallel.

The separate dirty bitmap also has the nice effect that forced screen
updates can be done cleanly by simply tagging the area in both guest and
server dirty map.  The old, hackish way was memset(old_data, 42, size)
to trick the code checking for screen changes.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>


git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@6860 c046a42c-6fe2-441c-8c8c-71466251a162
Showing 3 changed files with 105 additions and 85 deletions
... ... @@ -263,6 +263,7 @@ static inline int vnc_and_bits(const uint32_t *d1, const uint32_t *d2,
263 263  
264 264 static void vnc_update(VncState *vs, int x, int y, int w, int h)
265 265 {
  266 + struct VncSurface *s = &vs->guest;
266 267 int i;
267 268  
268 269 h += y;
... ... @@ -274,14 +275,14 @@ static void vnc_update(VncState *vs, int x, int y, int w, int h)
274 275 w += (x % 16);
275 276 x -= (x % 16);
276 277  
277   - x = MIN(x, vs->serverds.width);
278   - y = MIN(y, vs->serverds.height);
279   - w = MIN(x + w, vs->serverds.width) - x;
280   - h = MIN(h, vs->serverds.height);
  278 + x = MIN(x, s->ds->width);
  279 + y = MIN(y, s->ds->height);
  280 + w = MIN(x + w, s->ds->width) - x;
  281 + h = MIN(h, s->ds->height);
281 282  
282 283 for (; y < h; y++)
283 284 for (i = 0; i < w; i += 16)
284   - vnc_set_bit(vs->dirty_row[y], (x + i) / 16);
  285 + vnc_set_bit(s->dirty[y], (x + i) / 16);
285 286 }
286 287  
287 288 static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h)
... ... @@ -341,22 +342,17 @@ void buffer_append(Buffer *buffer, const void *data, size_t len)
341 342 static void vnc_resize(VncState *vs)
342 343 {
343 344 DisplayState *ds = vs->ds;
344   -
345 345 int size_changed;
346 346  
347   - vs->old_data = qemu_realloc(vs->old_data, ds_get_linesize(ds) * ds_get_height(ds));
348   -
349   - if (vs->old_data == NULL) {
350   - fprintf(stderr, "vnc: memory allocation failed\n");
351   - exit(1);
352   - }
353   -
354   - if (ds_get_bytes_per_pixel(ds) != vs->serverds.pf.bytes_per_pixel)
  347 + /* guest surface */
  348 + if (!vs->guest.ds)
  349 + vs->guest.ds = qemu_mallocz(sizeof(*vs->guest.ds));
  350 + if (ds_get_bytes_per_pixel(ds) != vs->guest.ds->pf.bytes_per_pixel)
355 351 console_color_init(ds);
356 352 vnc_colordepth(vs);
357   - size_changed = ds_get_width(ds) != vs->serverds.width ||
358   - ds_get_height(ds) != vs->serverds.height;
359   - vs->serverds = *(ds->surface);
  353 + size_changed = ds_get_width(ds) != vs->guest.ds->width ||
  354 + ds_get_height(ds) != vs->guest.ds->height;
  355 + *(vs->guest.ds) = *(ds->surface);
360 356 if (size_changed) {
361 357 if (vs->csock != -1 && vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
362 358 vnc_write_u8(vs, 0); /* msg id */
... ... @@ -367,9 +363,21 @@ static void vnc_resize(VncState *vs)
367 363 vnc_flush(vs);
368 364 }
369 365 }
  366 + memset(vs->guest.dirty, 0xFF, sizeof(vs->guest.dirty));
370 367  
371   - memset(vs->dirty_row, 0xFF, sizeof(vs->dirty_row));
372   - memset(vs->old_data, 42, ds_get_linesize(vs->ds) * ds_get_height(vs->ds));
  368 + /* server surface */
  369 + if (!vs->server.ds) {
  370 + vs->server.ds = default_allocator.create_displaysurface(ds_get_width(ds),
  371 + ds_get_height(ds));
  372 + } else {
  373 + default_allocator.resize_displaysurface(vs->server.ds,
  374 + ds_get_width(ds), ds_get_height(ds));
  375 + }
  376 + if (vs->server.ds->data == NULL) {
  377 + fprintf(stderr, "vnc: memory allocation failed\n");
  378 + exit(1);
  379 + }
  380 + memset(vs->server.dirty, 0xFF, sizeof(vs->guest.dirty));
373 381 }
374 382  
375 383 static void vnc_dpy_resize(DisplayState *ds)
... ... @@ -393,12 +401,12 @@ static void vnc_convert_pixel(VncState *vs, uint8_t *buf, uint32_t v)
393 401 {
394 402 uint8_t r, g, b;
395 403  
396   - r = ((((v & vs->serverds.pf.rmask) >> vs->serverds.pf.rshift) << vs->clientds.pf.rbits) >>
397   - vs->serverds.pf.rbits);
398   - g = ((((v & vs->serverds.pf.gmask) >> vs->serverds.pf.gshift) << vs->clientds.pf.gbits) >>
399   - vs->serverds.pf.gbits);
400   - b = ((((v & vs->serverds.pf.bmask) >> vs->serverds.pf.bshift) << vs->clientds.pf.bbits) >>
401   - vs->serverds.pf.bbits);
  404 + r = ((((v & vs->server.ds->pf.rmask) >> vs->server.ds->pf.rshift) << vs->clientds.pf.rbits) >>
  405 + vs->server.ds->pf.rbits);
  406 + g = ((((v & vs->server.ds->pf.gmask) >> vs->server.ds->pf.gshift) << vs->clientds.pf.gbits) >>
  407 + vs->server.ds->pf.gbits);
  408 + b = ((((v & vs->server.ds->pf.bmask) >> vs->server.ds->pf.bshift) << vs->clientds.pf.bbits) >>
  409 + vs->server.ds->pf.bbits);
402 410 v = (r << vs->clientds.pf.rshift) |
403 411 (g << vs->clientds.pf.gshift) |
404 412 (b << vs->clientds.pf.bshift);
... ... @@ -436,7 +444,7 @@ static void vnc_write_pixels_generic(VncState *vs, void *pixels1, int size)
436 444 {
437 445 uint8_t buf[4];
438 446  
439   - if (vs->serverds.pf.bytes_per_pixel == 4) {
  447 + if (vs->server.ds->pf.bytes_per_pixel == 4) {
440 448 uint32_t *pixels = pixels1;
441 449 int n, i;
442 450 n = size >> 2;
... ... @@ -444,7 +452,7 @@ static void vnc_write_pixels_generic(VncState *vs, void *pixels1, int size)
444 452 vnc_convert_pixel(vs, buf, pixels[i]);
445 453 vnc_write(vs, buf, vs->clientds.pf.bytes_per_pixel);
446 454 }
447   - } else if (vs->serverds.pf.bytes_per_pixel == 2) {
  455 + } else if (vs->server.ds->pf.bytes_per_pixel == 2) {
448 456 uint16_t *pixels = pixels1;
449 457 int n, i;
450 458 n = size >> 1;
... ... @@ -452,7 +460,7 @@ static void vnc_write_pixels_generic(VncState *vs, void *pixels1, int size)
452 460 vnc_convert_pixel(vs, buf, pixels[i]);
453 461 vnc_write(vs, buf, vs->clientds.pf.bytes_per_pixel);
454 462 }
455   - } else if (vs->serverds.pf.bytes_per_pixel == 1) {
  463 + } else if (vs->server.ds->pf.bytes_per_pixel == 1) {
456 464 uint8_t *pixels = pixels1;
457 465 int n, i;
458 466 n = size;
... ... @@ -470,7 +478,7 @@ static void send_framebuffer_update_raw(VncState *vs, int x, int y, int w, int h
470 478 int i;
471 479 uint8_t *row;
472 480  
473   - row = ds_get_data(vs->ds) + y * ds_get_linesize(vs->ds) + x * ds_get_bytes_per_pixel(vs->ds);
  481 + row = vs->server.ds->data + y * ds_get_linesize(vs->ds) + x * ds_get_bytes_per_pixel(vs->ds);
474 482 for (i = 0; i < h; i++) {
475 483 vs->write_pixels(vs, row, w * ds_get_bytes_per_pixel(vs->ds));
476 484 row += ds_get_linesize(vs->ds);
... ... @@ -519,8 +527,8 @@ static void send_framebuffer_update_hextile(VncState *vs, int x, int y, int w, i
519 527 int has_fg, has_bg;
520 528 uint8_t *last_fg, *last_bg;
521 529  
522   - last_fg = (uint8_t *) qemu_malloc(vs->serverds.pf.bytes_per_pixel);
523   - last_bg = (uint8_t *) qemu_malloc(vs->serverds.pf.bytes_per_pixel);
  530 + last_fg = (uint8_t *) qemu_malloc(vs->server.ds->pf.bytes_per_pixel);
  531 + last_bg = (uint8_t *) qemu_malloc(vs->server.ds->pf.bytes_per_pixel);
524 532 has_fg = has_bg = 0;
525 533 for (j = y; j < (y + h); j += 16) {
526 534 for (i = x; i < (x + w); i += 16) {
... ... @@ -673,16 +681,17 @@ static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int
673 681 }
674 682 }
675 683  
676   -static int find_dirty_height(VncState *vs, int y, int last_x, int x)
  684 +static int find_and_clear_dirty_height(struct VncSurface *s,
  685 + int y, int last_x, int x)
677 686 {
678 687 int h;
679 688  
680   - for (h = 1; h < (vs->serverds.height - y); h++) {
  689 + for (h = 1; h < (s->ds->height - y) && h < 1; h++) {
681 690 int tmp_x;
682   - if (!vnc_get_bit(vs->dirty_row[y + h], last_x))
  691 + if (!vnc_get_bit(s->dirty[y + h], last_x))
683 692 break;
684 693 for (tmp_x = last_x; tmp_x < x; tmp_x++)
685   - vnc_clear_bit(vs->dirty_row[y + h], tmp_x);
  694 + vnc_clear_bit(s->dirty[y + h], tmp_x);
686 695 }
687 696  
688 697 return h;
... ... @@ -693,8 +702,9 @@ static void vnc_update_client(void *opaque)
693 702 VncState *vs = opaque;
694 703 if (vs->need_update && vs->csock != -1) {
695 704 int y;
696   - uint8_t *row;
697   - char *old_row;
  705 + uint8_t *guest_row;
  706 + uint8_t *server_row;
  707 + int cmp_bytes = 16 * ds_get_bytes_per_pixel(vs->ds);
698 708 uint32_t width_mask[VNC_DIRTY_WORDS];
699 709 int n_rectangles;
700 710 int saved_offset;
... ... @@ -702,37 +712,37 @@ static void vnc_update_client(void *opaque)
702 712  
703 713 vga_hw_update();
704 714  
  715 + /*
  716 + * Walk through the guest dirty map.
  717 + * Check and copy modified bits from guest to server surface.
  718 + * Update server dirty map.
  719 + */
705 720 vnc_set_bits(width_mask, (ds_get_width(vs->ds) / 16), VNC_DIRTY_WORDS);
706   -
707   - /* Walk through the dirty map and eliminate tiles that
708   - really aren't dirty */
709   - row = ds_get_data(vs->ds);
710   - old_row = vs->old_data;
711   -
712   - for (y = 0; y < ds_get_height(vs->ds); y++) {
713   - if (vnc_and_bits(vs->dirty_row[y], width_mask, VNC_DIRTY_WORDS)) {
  721 + guest_row = vs->guest.ds->data;
  722 + server_row = vs->server.ds->data;
  723 + for (y = 0; y < vs->guest.ds->height; y++) {
  724 + if (vnc_and_bits(vs->guest.dirty[y], width_mask, VNC_DIRTY_WORDS)) {
714 725 int x;
715   - uint8_t *ptr;
716   - char *old_ptr;
717   -
718   - ptr = row;
719   - old_ptr = (char*)old_row;
720   -
721   - for (x = 0; x < ds_get_width(vs->ds); x += 16) {
722   - if (memcmp(old_ptr, ptr, 16 * ds_get_bytes_per_pixel(vs->ds)) == 0) {
723   - vnc_clear_bit(vs->dirty_row[y], (x / 16));
724   - } else {
725   - has_dirty = 1;
726   - memcpy(old_ptr, ptr, 16 * ds_get_bytes_per_pixel(vs->ds));
727   - }
728   -
729   - ptr += 16 * ds_get_bytes_per_pixel(vs->ds);
730   - old_ptr += 16 * ds_get_bytes_per_pixel(vs->ds);
  726 + uint8_t *guest_ptr;
  727 + uint8_t *server_ptr;
  728 +
  729 + guest_ptr = guest_row;
  730 + server_ptr = server_row;
  731 +
  732 + for (x = 0; x < vs->guest.ds->width;
  733 + x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
  734 + if (!vnc_get_bit(vs->guest.dirty[y], (x / 16)))
  735 + continue;
  736 + vnc_clear_bit(vs->guest.dirty[y], (x / 16));
  737 + if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0)
  738 + continue;
  739 + memcpy(server_ptr, guest_ptr, cmp_bytes);
  740 + vnc_set_bit(vs->server.dirty[y], (x / 16));
  741 + has_dirty++;
731 742 }
732 743 }
733   -
734   - row += ds_get_linesize(vs->ds);
735   - old_row += ds_get_linesize(vs->ds);
  744 + guest_row += ds_get_linesize(vs->ds);
  745 + server_row += ds_get_linesize(vs->ds);
736 746 }
737 747  
738 748 if (!has_dirty && !vs->audio_cap) {
... ... @@ -740,25 +750,30 @@ static void vnc_update_client(void *opaque)
740 750 return;
741 751 }
742 752  
743   - /* Count rectangles */
  753 + /*
  754 + * Send screen updates to the vnc client using the server
  755 + * surface and server dirty map. guest surface updates
  756 + * happening in parallel don't disturb us, the next pass will
  757 + * send them to the client.
  758 + */
744 759 n_rectangles = 0;
745 760 vnc_write_u8(vs, 0); /* msg id */
746 761 vnc_write_u8(vs, 0);
747 762 saved_offset = vs->output.offset;
748 763 vnc_write_u16(vs, 0);
749 764  
750   - for (y = 0; y < vs->serverds.height; y++) {
  765 + for (y = 0; y < vs->server.ds->height; y++) {
751 766 int x;
752 767 int last_x = -1;
753   - for (x = 0; x < vs->serverds.width / 16; x++) {
754   - if (vnc_get_bit(vs->dirty_row[y], x)) {
  768 + for (x = 0; x < vs->server.ds->width / 16; x++) {
  769 + if (vnc_get_bit(vs->server.dirty[y], x)) {
755 770 if (last_x == -1) {
756 771 last_x = x;
757 772 }
758   - vnc_clear_bit(vs->dirty_row[y], x);
  773 + vnc_clear_bit(vs->server.dirty[y], x);
759 774 } else {
760 775 if (last_x != -1) {
761   - int h = find_dirty_height(vs, y, last_x, x);
  776 + int h = find_and_clear_dirty_height(&vs->server, y, last_x, x);
762 777 send_framebuffer_update(vs, last_x * 16, y, (x - last_x) * 16, h);
763 778 n_rectangles++;
764 779 }
... ... @@ -766,7 +781,7 @@ static void vnc_update_client(void *opaque)
766 781 }
767 782 }
768 783 if (last_x != -1) {
769   - int h = find_dirty_height(vs, y, last_x, x);
  784 + int h = find_and_clear_dirty_height(&vs->server, y, last_x, x);
770 785 send_framebuffer_update(vs, last_x * 16, y, (x - last_x) * 16, h);
771 786 n_rectangles++;
772 787 }
... ... @@ -895,9 +910,10 @@ int vnc_client_io_error(VncState *vs, int ret, int last_errno)
895 910 if (!vs->vd->clients)
896 911 dcl->idle = 1;
897 912  
898   - qemu_free(vs->old_data);
  913 + default_allocator.free_displaysurface(vs->server.ds);
  914 + qemu_free(vs->guest.ds);
899 915 qemu_free(vs);
900   -
  916 +
901 917 return 0;
902 918 }
903 919 return ret;
... ... @@ -1392,13 +1408,11 @@ static void framebuffer_update_request(VncState *vs, int incremental,
1392 1408 int i;
1393 1409 vs->need_update = 1;
1394 1410 if (!incremental) {
1395   - char *old_row = vs->old_data + y_position * ds_get_linesize(vs->ds);
1396   -
1397 1411 for (i = 0; i < h; i++) {
1398   - vnc_set_bits(vs->dirty_row[y_position + i],
  1412 + vnc_set_bits(vs->guest.dirty[y_position + i],
  1413 + (ds_get_width(vs->ds) / 16), VNC_DIRTY_WORDS);
  1414 + vnc_set_bits(vs->server.dirty[y_position + i],
1399 1415 (ds_get_width(vs->ds) / 16), VNC_DIRTY_WORDS);
1400   - memset(old_row, 42, ds_get_width(vs->ds) * ds_get_bytes_per_pixel(vs->ds));
1401   - old_row += ds_get_linesize(vs->ds);
1402 1416 }
1403 1417 }
1404 1418 }
... ... @@ -1526,7 +1540,7 @@ static void set_pixel_format(VncState *vs,
1526 1540 return;
1527 1541 }
1528 1542  
1529   - vs->clientds = vs->serverds;
  1543 + vs->clientds = *(vs->guest.ds);
1530 1544 vs->clientds.pf.rmax = red_max;
1531 1545 count_bits(vs->clientds.pf.rbits, red_max);
1532 1546 vs->clientds.pf.rshift = red_shift;
... ... @@ -1980,8 +1994,6 @@ static void vnc_connect(VncDisplay *vd, int csock)
1980 1994 vnc_write(vs, "RFB 003.008\n", 12);
1981 1995 vnc_flush(vs);
1982 1996 vnc_read_when(vs, protocol_version, 12);
1983   - memset(vs->old_data, 0, ds_get_linesize(vs->ds) * ds_get_height(vs->ds));
1984   - memset(vs->dirty_row, 0xFF, sizeof(vs->dirty_row));
1985 1997 vnc_update_client(vs);
1986 1998 reset_keys(vs);
1987 1999  
... ...
... ... @@ -104,15 +104,23 @@ struct VncDisplay
104 104 #endif
105 105 };
106 106  
  107 +struct VncSurface
  108 +{
  109 + uint32_t dirty[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS];
  110 + DisplaySurface *ds;
  111 +};
  112 +
107 113 struct VncState
108 114 {
109 115 QEMUTimer *timer;
110 116 int csock;
  117 +
111 118 DisplayState *ds;
  119 + struct VncSurface guest; /* guest visible surface (aka ds->surface) */
  120 + struct VncSurface server; /* vnc server surface */
  121 +
112 122 VncDisplay *vd;
113 123 int need_update;
114   - uint32_t dirty_row[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS];
115   - char *old_data;
116 124 uint32_t features;
117 125 int absolute;
118 126 int last_x;
... ... @@ -138,7 +146,7 @@ struct VncState
138 146 /* current output mode information */
139 147 VncWritePixels *write_pixels;
140 148 VncSendHextileTile *send_hextile_tile;
141   - DisplaySurface clientds, serverds;
  149 + DisplaySurface clientds;
142 150  
143 151 CaptureVoiceOut *audio_cap;
144 152 struct audsettings as;
... ...
vnchextile.h
... ... @@ -13,7 +13,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
13 13 void *last_fg_,
14 14 int *has_bg, int *has_fg)
15 15 {
16   - uint8_t *row = (ds_get_data(vs->ds) + y * ds_get_linesize(vs->ds) + x * ds_get_bytes_per_pixel(vs->ds));
  16 + uint8_t *row = vs->server.ds->data + y * ds_get_linesize(vs->ds) + x * ds_get_bytes_per_pixel(vs->ds);
17 17 pixel_t *irow = (pixel_t *)row;
18 18 int j, i;
19 19 pixel_t *last_bg = (pixel_t *)last_bg_;
... ...