wined3d: universal surface convertor function for unsigned integer color formats

Victor ErV2005 at rambler.ru
Mon Jul 14 11:41:29 CDT 2008


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.

-- 
Виктор Ерёмин (ErV2005 at rambler.ru)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: This is a digitally signed message part.
Url : http://www.winehq.org/pipermail/wine-devel/attachments/20080714/e7fe8785/attachment.pgp 


More information about the wine-devel mailing list