From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MfZVQ-0002fF-Fs for qemu-devel@nongnu.org; Mon, 24 Aug 2009 09:22:40 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MfZVL-0002a2-3J for qemu-devel@nongnu.org; Mon, 24 Aug 2009 09:22:39 -0400 Received: from [199.232.76.173] (port=54893 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MfZVK-0002Zr-VM for qemu-devel@nongnu.org; Mon, 24 Aug 2009 09:22:34 -0400 Received: from mail.gmx.net ([213.165.64.20]:35880) by monty-python.gnu.org with smtp (Exim 4.60) (envelope-from ) id 1MfZVK-00067L-8Z for qemu-devel@nongnu.org; Mon, 24 Aug 2009 09:22:34 -0400 Date: Mon, 24 Aug 2009 15:22:29 +0200 From: Reimar =?iso-8859-1?Q?D=F6ffinger?= Subject: Re: [Qemu-devel] [PATCH] use corect depth from DisplaySurface in vmware_vga.c Message-ID: <20090824132229.GB24730@1und1.de> References: <20090817100828.GA22029@1und1.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Sun, Aug 23, 2009 at 07:25:32PM +0200, andrzej zaborowski wrote: > 2009/8/17 Reimar Döffinger : > > Hello, > > for what I can tell, there is no way for vmware_vga to work correctly > > right now. It assumes that the framebuffer bits-per-pixel and the one > > from the DisplaySurface are identical (it uses directly the VRAM from > > vga.c), but it always assumes 3 bytes per pixel, which is never possible > > with the current version of DisplaySurface. > > Attached patch fixes that by using ds_get_bits_per_pixel. > > 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. As such I claim that my patch is correct, it may uncover some bugs that were very carefully swept under the rug, but that only makes it incomplete. Also a reference to either the previous discussion or at least a proper "bug report" about what/how/where it breaks with my patch applied would be very helpful, e.g. which operating system/hardware driver/SDL version (I guess there is some reason why I get a different bit depth). As such, I want to add that the revert commit message of "was incorrect." doesn't qualify as useful to me. > 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? 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. (Though looking at sdl_setdata/sdl_update some conversion should be done anyway, though that requires that the values in the DisplaySurface and in the vmware_vga depth variable match, which at least at some points in time they currently don't). > Unfortunately the ability to know host's pixel depth was dropped > during video API conversion and afaik hasn't been added till now. Considering the above, I think that might count as an "accidentally good" decision. Greetings, Reimar Döffinger