Re: [Qemu-devel] [PATCH] use corect depth from DisplaySurface in vmware_vga.c

[andrzej zaborowski]

2009/8/24 Reimar Döffinger <address@hidden>:
> On Sun, Aug 23, 2009 at 07:25:32PM +0200, andrzej zaborowski wrote:
>> It was discussed at some point earlier that at the time this code runs
>> SDL is not initialised and the depth returned is an arbitrary value
>> from default allocator.
>
> It is not arbitrary. It matches exactly the DisplaySurface's
> linesize and data buffer size.

Only at the moment the function is called.  The value is still
hardcoded, just elsewhere.  Once display backend initialises this
value may be invalid.

> As such, I want to add that the revert commit message of "was
> incorrect." doesn't qualify as useful to me.

I wasn't intending to push this commit, instead I responded to the
thread but later noticed I had pushed it.

>
>> What vmware_vga really should do is ask SDL
>> for the host's depth and set the surface's pixelformat to that.
>
> Obvious question: why shouldn't SDL ask the VGA for its depth and try
> to use a surface with that format?

It should, the VGA should create the surface using
qemu_create_displaysurface... like in vga.c.  But, this depth is not
set by the guest, it should match the host's depth because this is how
the vmware's "specification" (if you can call it that) defines it.

Beside that it's an obvious performance gain.  The API change did not
magically remove the pixel by pixel conversion of the colour space, it
just hid it in SDL, under more indirection.

> Has the advantage that the depth
> of the emulated stuff stays the same, whereas with your suggestion
> if I tried a loadvm from a savevm of your machine qemu would get
> in a bit in trouble.

I'm not sure I understand this sentence, but apparently there's some
way vmware can communicate to the guest that the bit depth has
changed.  This is not implemented in vmware_vga.c yet.

Similarly when the window is resized, instead of zooming we could
communicate the resolution change to the guest, but it's not
implemented yet.

Cheers