[Mesa-dev] [PATCH 2/2] mesa: Speedup the xrgb -> argb special case in fast_read_rgba_pixels_memcpy

[Ian Romanick]

On 03/11/2013 07:56 AM, Jose Fonseca wrote:
> I'm surprised this is is faster.
>
> In particular, for big things we'll be touching memory twice.
>
> Did you measure the speed up?

The second hit is cache-hot, so it may not be too expensive.  I suspect 
memcpy is optimized to fill the cache in a more efficient manner than 
the old loop.  Since the old loop did a read and a bit-wise or, it's 
also possible the compiler generated some really dumb code.  We'd have 
to look at the assembly output to know.

As Patrick suggests, there's probably an SSE2 method to do this even 
faster.  That may be worth investigating.

Once upon a time Matt Turner was talking about using pixman to 
accelerate operations like this in Mesa.  It has a lot of highly 
optimized paths for just this sort of thing.  Since it's used by other 
projects, it gets a lot more testing, etc.  It may be worth looking at 
using that to solve this problem.

> Jose
>
> ----- Original Message -----
>> ---
>>   src/mesa/main/readpix.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c
>> index 349b0bc..0f5c84c 100644
>> --- a/src/mesa/main/readpix.c
>> +++ b/src/mesa/main/readpix.c
>> @@ -285,11 +285,12 @@ fast_read_rgba_pixels_memcpy( struct gl_context *ctx,
>>         }
>>      } else if (copy_xrgb) {
>>         /* convert xrgb -> argb */
>> +      int alphaOffset = texelBytes - 1;
>>         for (j = 0; j < height; j++) {
>> -         GLuint *dst4 = (GLuint *) dst, *map4 = (GLuint *) map;
>> +         memcpy(dst, map, width * texelBytes);
>>            int i;

At the very least, the declaration needs to be moved before the memcpy 
or it will break the build on Windows.

>>            for (i = 0; i < width; i++) {
>> -            dst4[i] = map4[i] | 0xff000000;  /* set A=0xff */
>> +            dst[i * texelBytes + alphaOffset] = 0xff;  /* set A=0xff */
>>            }
>>            dst += dstStride;
>>            map += stride;
>> --
>> 1.8.1.5
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev