There's a pixelformat table that describes bitmasks and sizes in utils.c, you can access it with a function that is next to it. I think you can infer the shifting information from this table, if not please add it there instead of creating your own table.
Also please do not use C++ comments("//"), and watch out regarding tabs and spaces
Your patch still adds the x8r8g8b8_to_x5r6g5 function, and I think it is a bit ugly to bypass the existing format conversion table entirely and add the convert_unsigned_pixels function. I think it would be better to access this function using the conversion table. One reason is that we could use the table in directx.c in CheckDeviceFormatConversion, so the information is all in one place.
-----Original Message----- From: wine-patches-bounces@winehq.org [mailto:wine-patches- bounces@winehq.org] On Behalf Of Victor Eremin Sent: Sunday, July 13, 2008 5:52 PM To: wine-patches@winehq.org Subject: wined3d: universal surface convertor function for unsigned integer color formats
Converter function supports conversion between most of "unsigned color" argb/xrgb surface formats (like D3DFMT_A8R8G8B8, D3DFMT_A8R3G3B2, and so on), and between "luminosity" color formats (D3DFMT_L8, etc), excluding D3DFMT_A16R16G16B16, D3DFMT_A8P8, D3DFMT_P8 and D3DFMT_A1.
Conversion from rgba to luminosity (and vice versa) is not currently implemented.
The patch removes "Cannot find coverter" FIXMEs from "Ancient Evil" and "Stranded 2" games.
Patch also fixes water glitches in "Stranded 2" game.
dlls/wined3d/surface_base.c | 215 +++++++++++++++++++++++++++++++++++++++++- 1 files changed, 209 insertions(+), 6 deletions(-)
On Monday 14 July 2008 18:21:19 Stefan Dösinger wrote:
There's a pixelformat table that describes bitmasks and sizes in utils.c, you can access it with a function that is next to it. I think you can infer the shifting information from this table, if not please add it there instead of creating your own table.
I've found this array (StaticPixelFormatDesc formats[]), but I'm not sure if adding new fields will be a good idea. Reasons: 1) This will contain redundant and unused information (masks<->shifts. I wanted to use masks initially, but decided to use "mask_size" and "shift", because it's less prone to errors). Redundant means higher chance of errors and breaking up something somewhere else. 2) Ideally for conversion purposes it should have more channel fields. (at least "L" (the only one currently used), but ideally also bitmasks for "W", "V", "U", "D", "S" channels). But adding more fields will easily turn formats[] table into monstrosity. 3) StaticFormatDescriptor (and my own "SurfFmtDesc") both aren't really what I'd like to use with conversion routine. Ideally something like that: --- struct ChannelDesc{ char name; /*channel neame 'a', 'r', 'g', 'd', etc*/ BYTE bits; BYTE shift; int defaultBits; /*if 1 then during conversion channel bits in destination surface must be set to 1, if such channel doesn't exist in source surface.*/ }
struct FormatDesc{ WINED3DFORMAT fmt; BYTE size; BYTE numChannels; struct ChannelDesc channels[8]; } --- could be perfect/compact descriptor suitable for converting surfaces. This way it could be much easier to handle more surface format conversions in simple fashion. I.e. given two such descriptors I could put all conversion routine in a loop that would handle almost anything, instead of calling same function several times for each channel. Such function could be useful in some other places.
Is it possible to accept patch without merging two tables, until better (or "final") conversion implementation will be written? (like the one which handles A8R8G8B8->L8 or A8R8G8B8->P8 conversions) Your arguments about C++ comments/spaces, unused x8r8g8b8_to_x5r6g5 are reasonable, but I'm not sure that merging current table used by convert_unsigned_pixels() with "StaticPixelFormatDesc formats[]" from utils.c will be good idea.
Also please do not use C++ comments("//"), and watch out regarding tabs and spaces
Will do.
Your patch still adds the x8r8g8b8_to_x5r6g5 function, and I think it is a bit ugly to bypass the existing format conversion table entirely and add the convert_unsigned_pixels function. I think it would be better to access this function using the conversion table. One reason is that we could use the table in directx.c in CheckDeviceFormatConversion, so the information is all in one place.
Currently default conversion (find_convertor) is not bypassed. It is still used if there is no conversion available via convert_unsigned_pixels(), which is likely to happen, because convert_unsigned_pixels() supports only 20 formats. Also, please note that there is only one entry in "convertors" table currently (if you don't count x8r8g8b8_to_x5r6g5). putting convert_unsigned_pixels in "convertors" table won't be possible, because it'll require either writing of 400 wrappers and adding 400 entries in converter table (unless I don't know some C-technique for avoiding this). This is extremely ugly. Also, currently it won't be possible to support all conversions using convertors table. You won't be able to use current convertors for formats with palette (someone already have sent me email asking to implement X8R8G8B8->P8 conversion). I think instead of using only conversion table it will be better to provide centrilised functions like "is_conversion_supported(WINED3DFORMAT, WINED3DFORMAT)" and "convert_pixels". It might make sense to use different routines/tables for different format conversions.
-
Stefan Dösinger -
Victor