🔒 Locked

byte swapping optimization

Started by ralphinator Aug 11, 2010 at 2:31 PM 6 replies 1.9k views

Original Post

Aug 11, 2010 at 2:31 PM

While using the code to load uncompressed .tga files (NEHE lesson #33) I get bluish green image using the optimized xor method but no problems while using the traditional byte swapping method. the two methods or outlined below.

I verified the math for the optimized byte swapping using xor is correct. Does anybody have any ideas on what might cause this?

Thanks!

// -- Optimized method// Byte Swapping Optimized By Steve Thomasfor(GLuint cswap = 0; cswap < (int)tga.imageSize; cswap += tga.bytesPerPixel){    texture->imageData[cswap] ^= texture->imageData[cswap+2] ^=    texture->imageData[cswap] ^= texture->imageData[cswap+2];}

// -- Traditional methodfor(GLuint i=0; i<int(tga.imageSize); i+=tga.bytesPerPixel) // Loop Through The Image Data{ // Swaps The 1st And 3rd Bytes ('R'ed and 'B'lue)    tga.temp=texture->imageData; // Temporarily Store The Value At Image Data 'i'    texture->imageData = texture->imageData[i + 2]; // Set The 1st Byte To The Value Of The 3rd Byte    texture->imageData[i + 2] = tga.temp; // Set The 3rd Byte To The Value In 'temp' (1st Byte Value)}

Palidine

Aug 11, 2010 at 2:50 PM

Define "optimized". Did you actually find that this method was a bottleneck in your program? Did profiling after the change actually demonstrate a performance boost? Does the xor method actually generate a shorter assembly instruction set? I think it's at best the same, and potentially slower to use the xor method.

Also, why is your temp variable a member variable? It should be a stack variable:

for(GLuint i=0; i<int(tga.imageSize); i+=tga.bytesPerPixel) // Loop Through The Image Data{ // Swaps The 1st And 3rd Bytes ('R'ed and 'B'lue)    unsigned char temp = texture->imageData;    texture->imageData = texture->imageData[i + 2];    texture->imageData[i + 2] = temp;}

-me

Halifax2

Aug 11, 2010 at 3:03 PM

On old systems when bitwise manipulation was faster than loads and stores, you are probably correct, that code would be optimized. But as Palidine pointed out, these days it's negligible.

At any rate, you short-handed this to your above version:

texture->imageData[cswap] ^= texture->imageData[cswap+2];texture->imageData[cswap+2] ^= texture->imageData[cswap];texture->imageData[cswap] ^= texture->imageData[cswap+2];

The behavior of the above is defined, the behavior of short-hand version is not defined due to the modification of texture->imageData[cswap] twice without a sequence point, thus manifesting itself in undefined behavior.

I recommend you read up on sequence points.

ralphinator

Aug 11, 2010 at 4:25 PM

Thanks for the quick replies and I agree that the term 'optimized' is suspect for that piece of code on todays machines. I simply pulled the code and the term from lesson #33 of the NEHE tutorials.

In any case I think Halifax2 is right about the lack of a sequence point. I'll test that later today and post the results.

ralphinator

Aug 11, 2010 at 6:00 PM

Halifax2 your solution was correct thanks!

Halifax2

Aug 11, 2010 at 9:46 PM

Quote:
Original post by ralphinator
Halifax2 your solution was correct thanks!

Glad to know I was helpful for once. =)

elFarto

Aug 12, 2010 at 11:20 AM

Isn't the XOR swap method slower on modern CPUs, due to the dependency created between the instructions forcing it to execute them in order.

Regards
elFarto

MarkS

Aug 12, 2010 at 12:13 PM

Quote:
Original post by Halifax2
I recommend you read up on sequence points.

Thank you for this! I have never heard of that term before and knowing about it solves quite a bit of issues I've had in the past.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.