[Mesa-dev] [PATCH 2/2] mesa: fix unpack_ARGB1555_REV()

[Brian Paul]

On 12/01/2011 02:42 AM, Michel Dänzer wrote:
> On Mit, 2011-11-30 at 20:36 -0700, Brian Paul wrote:
>> We weren't doing the necessary byte swap.
>
> I just stumbled over this as well.
>
>
>> ---
>>   src/mesa/main/format_unpack.c |    9 +++++----
>>   1 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/main/format_unpack.c b/src/mesa/main/format_unpack.c
>> index 4b4ee6b..fc0db34 100644
>> --- a/src/mesa/main/format_unpack.c
>> +++ b/src/mesa/main/format_unpack.c
>> @@ -275,10 +275,11 @@ unpack_ARGB1555_REV(const void *src, GLfloat dst[][4], GLuint n)
>>      const GLushort *s = ((const GLushort *) src);
>>      GLuint i;
>>      for (i = 0; i<  n; i++) {
>> -      dst[i][RCOMP] = UBYTE_TO_FLOAT( ((s[i]>>   7)&  0xf8) | ((s[i]>>  12)&  0x7) );
>> -      dst[i][GCOMP] = UBYTE_TO_FLOAT( ((s[i]>>   2)&  0xf8) | ((s[i]>>   7)&  0x7) );
>> -      dst[i][BCOMP] = UBYTE_TO_FLOAT( ((s[i]<<   3)&  0xf8) | ((s[i]>>   2)&  0x7) );
>> -      dst[i][ACOMP] = UBYTE_TO_FLOAT( ((s[i]>>  15)&  0x01) * 255 );
>> +      GLushort tmp = (s[i]<<  8) | (s[i]>>  8); /* byteswap */
>
> A BSWAP16() macro might be nice for this, but I wouldn't let that hold
> up this fix.
>
>> +      dst[i][RCOMP] = UBYTE_TO_FLOAT( ((tmp>>   7)&  0xf8) | ((tmp>>  12)&  0x7) );
>> +      dst[i][GCOMP] = UBYTE_TO_FLOAT( ((tmp>>   2)&  0xf8) | ((tmp>>   7)&  0x7) );
>> +      dst[i][BCOMP] = UBYTE_TO_FLOAT( ((tmp<<   3)&  0xf8) | ((tmp>>   2)&  0x7) );
>
> Don't these lines need to be changed to be the same as in
> unpack_ARGB1555()?

It should be equivalent (lookup table vs. arithmetic).  It would nice 
if someone could write a little benchmark to see which is really faster.

I'll commit this later if there's no other concerns.

-Brian