Re: [Qemu-devel] [PATCH 1 of 6] [UPDATE] vnc dynamic resolution

[Anthony Liguori <anthony@codemonkey.ws>]

Stefano Stabellini wrote:
> This patch implements dynamic colour depth changes in vnc.c:
> this way the vnc server can change its own internal colour depth at run
> time to follow any guest resolution change.
>   

When testing this patch, gvncviewer hangs when attempting to connect.  
It's waiting to receive the ServerPixelFormat and it doesn't appear to 
receive it.

> @@ -1318,7 +1440,9 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
>      vnc_write_u16(vs, vs->ds->height);
>  
>      vnc_write_u8(vs, vs->depth * 8); /* bits-per-pixel */
> -    vnc_write_u8(vs, vs->depth * 8); /* depth */
> +    if (vs->depth == 4) vnc_write_u8(vs, 24); /* depth */
> +    else vnc_write_u8(vs, vs->depth * 8); /* depth */
>   

Why unconditionally change to a depth of 24 instead of 32?  There's no 
savings from a protocol perspective.

Regards,

Anthony Liguori

>  #ifdef WORDS_BIGENDIAN
>      vnc_write_u8(vs, 1);             /* big-endian-flag */
>  #else
> @@ -2008,7 +2132,6 @@ void vnc_display_init(DisplayState *ds)
>  
>      vs->lsock = -1;
>      vs->csock = -1;
> -    vs->depth = 4;
>      vs->last_x = -1;
>      vs->last_y = -1;
>  
> diff --git a/vnchextile.h b/vnchextile.h
> index 09c1b27..eb05feb 100644
> --- a/vnchextile.h
> +++ b/vnchextile.h
> @@ -2,29 +2,29 @@
>  #define CONCAT(a, b) CONCAT_I(a, b)
>  #define pixel_t CONCAT(uint, CONCAT(BPP, _t))
>  #ifdef GENERIC
> -#define NAME generic
> +#define NAME CONCAT(generic_, BPP)
>  #else
>  #define NAME BPP
>  #endif
>  
>  static void CONCAT(send_hextile_tile_, NAME)(VncState *vs,
>                                               int x, int y, int w, int h,
> -                                             uint32_t *last_bg32,
> -                                             uint32_t *last_fg32,
> +                                             void *last_bg_,
> +                                             void *last_fg_,
>                                               int *has_bg, int *has_fg)
>  {
>      uint8_t *row = (vs->ds->data + y * vs->ds->linesize + x * vs->depth);
>      pixel_t *irow = (pixel_t *)row;
>      int j, i;
> -    pixel_t *last_bg = (pixel_t *)last_bg32;
> -    pixel_t *last_fg = (pixel_t *)last_fg32;
> +    pixel_t *last_bg = (pixel_t *)last_bg_;
> +    pixel_t *last_fg = (pixel_t *)last_fg_;
>      pixel_t bg = 0;
>      pixel_t fg = 0;
>      int n_colors = 0;
>      int bg_count = 0;
>      int fg_count = 0;
>      int flags = 0;
> -    uint8_t data[(sizeof(pixel_t) + 2) * 16 * 16];
> +    uint8_t data[(vs->pix_bpp + 2) * 16 * 16];
>      int n_data = 0;
>      int n_subtiles = 0;
>  
>
>
>