RE: wined3d: universal surface convertor function for unsigned integer color formats(3rd attempt)

[Stefan Dösinger]

The patch looks reasonably, just one small thing: There is a count_bits function implemented in utils.c, which as far as I can see does the same as getMaskSize. Can you check if you can reuse it?
> -----Original Message-----
> From: [e-mail] [mailto:wine-patches-
> [e-mail]] On Behalf Of Victor Eremin
> Sent: Wednesday, July 23, 2008 10:25 AM
> To: [e-mail]
> Subject: wined3d: universal surface convertor function for unsigned
> integer color formats(3rd attempt)
>
> Supports conversion between most of "unsigned color" argb/xrgb surface
> formats (D3DFMT_A8R8G8B8, D3DFMT_A8R3G3B2, etc), and "luminosity" color
> formats (D3DFMT_L8, etc), excluding D3DFMT_A16R16G16B16, D3DFMT_A8P8,
> D3DFMT_P8 and D3DFMT_A1.
> luminosity to argb/xrgb (and vice versa) conversions are not supported
>
> Removes "Cannot find coverter" FIXMEs from "Ancient Evil" and "Stranded
> 2" games.
> Fixes water glitches in "Stranded 2" game.
>
> dlls/wined3d/surface_base.c    |  202
> ++++++++++++++++++++++++++++++++++++++-
>  dlls/wined3d/utils.c           |  126 +++++++++++++-------------
>  dlls/wined3d/wined3d_private.h |    2 +-
>  3 files changed, 260 insertions(+), 70 deletions(-)

wined3d: universal surface convertor function for unsigned integer color formats(3rd attempt)

[Chris Robinson]

On Wednesday 23 July 2008 09:10:02 am Stefan Dösinger wrote:
> The patch looks reasonably, just one small thing: There is a count_bits
> function implemented in utils.c, which as far as I can see does the same as
> getMaskSize. Can you check if you can reuse it?

Maybe it would be better if the table was changed to have a bit offset and a 
mask size, instead of the actual mask. All the bits for any given component 
are always continuous, and things would be easier to handle with the offset 
and size (eg. the float formats could get a proper mask; not that they could 
be converted in this way, though). So for a generic converter, you can 
basically get:

outcolor = (((incolor>>infmt->r_offset)&((1<<infmt->r_size)-1)) * 
outfmt->r_size / infmt->r_size) << outfmt->r_offset;
outcolor |= (((incolor>>infmt->g_offset)&((1<<infmt->g_size)-1)) * 
outfmt->g_size / infmt->g_size) << outfmt->g_offset;
outcolor |= (((incolor>>infmt->b_offset)&((1<<infmt->b_size)-1)) * 
outfmt->b_size / infmt->b_size) << outfmt->b_offset;
outcolor |= (((incolor>>infmt->a_offset)&((1<<infmt->a_size)-1)) * 
outfmt->a_size / infmt->a_size) << outfmt->a_offset;

For any non-mixed unsigned integer type.

wined3d: universal surface convertor function for unsigned integer color formats(3rd attempt)

[Victor]

On Wednesday 23 July 2008 21:18:57 Chris Robinson wrote:
> Maybe it would be better if the table was changed to have a bit offset and
> a mask size, instead of the actual mask.

0) initial patch used "mask size" + "mask offset", but was rewritten to use 
mask value when Stefan Dösinger requested that. I don't want to rewrite it 
back to use mask size + mask offset. 
1) mask size and offset can be extracted from mask value.
2) using mask instead of "mask size" + "mask offset" requires less function 
arguments and smaller format table, although, yes there is a higher chance of 
producing errors.

wined3d: universal surface convertor function for unsigned integer color formats(3rd attempt)

[Chris Robinson]

On Wednesday 23 July 2008 10:29:37 am Victor wrote:
> - initial patch used "mask size" + "mask offset", but was rewritten to use
>
> mask value when Stefan Dösinger requested that. I don't want to rewrite it
> back to use mask size + mask offset.
> - mask size and offset can be extracted from mask value.
> - using mask instead of "mask size" + "mask offset" requires less function
>
> arguments and smaller format table, although, yes there is a higher chance
> of producing errors.

But extracting the mask offset and size from the actual mask takes a bit of 
time, and as it is, the table can't currently set a proper mask for anything 
over 32 bits (including the 16-bit-per-component unsigned integer types).

wined3d: universal surface convertor function for unsigned integer color formats(3rd attempt)

[Victor]

On Wednesday 23 July 2008 21:48:59 Chris Robinson wrote:
> But extracting the mask offset and size from the actual mask takes a bit of
> time,

Extracting mask is called once or twice per channel conversion. I.e. in case 
of converting a8r8g8b8->a1r5g5b5 mask should be calculated only 8 times, no 
matter how big surface is. check mask_copy() routine for details. Optimizing 
this is pointless - converter performs much more per pixel operations - 
shifts, bitwise operations, etc, so you won't notice any difference.
> and as it is, the table can't currently set a proper mask for 
> anything over 32 bits (including the 16-bit-per-component unsigned integer
> types).

Yes, this isn't supported. But with current scheme of conversion, adding 
support for 16-bit-per-component surfaces would require operating on 64bit 
numbers or increasing number of per-pixel operations. I think per-pixel 64bit 
shifts on 32bit CPUs will be slower.

replacing masks with number of bits and shift will need additional work, since 
masks are probably used in other places. There will be also high chance of 
breaking entire table accidentally because of misprint.

If you don't like current functionality, to my opinion the best approach would 
be to modify it once my patch made it in repository.