Sign in to follow this  
3dmodelerguy

TGA file format question

Recommended Posts

ok I can open the TA file format but the colors and messed up. I know that the colors in the uncompressed TGA are stored as BGR and not RGB so i have this right fix it, or so i thought but colors are still messed up. what is wrond with this: for( GLuint i = 0; i < ( int )image_size; i += bytes_per_pixel ) { // stores the blue color bit GLuint temp = _image_data[ i ]; // switches the blue color bit with the red color bit _image_data[ i ] = _image_data[ i + 2 ]; // put the sotred blue color bits in the thrid spot where it should be _image_data[ i + 2 ] = temp; } I know that loops through every pixel but inside the loop, is everything ok?

Share this post


Link to post
Share on other sites
That's the code Ive got in my LoadTexture function. Perhaps the error is elsewhere?

Check the format being supplied to glTexImage2D (Is that what your using yeah?)

(Remeber its GL_RGB if 24bpp else GL_RGBA for 32bpp)

Share this post


Link to post
Share on other sites
It supposed to be BGR.

this will do it

_image_data[i] ^= _image_data[i+2] =^ image_data[i] ^= image_data[i+2];


that'll switch it to RGBA.

think of it this way.

A XOR B = AXORB
AXORB XOR A = B
AXORB XOR B = A;

make sure you test your bytes per pixel before you submit to gl.

Share this post


Link to post
Share on other sites
That XOR trick strikes me as premature optimisation to be honest. It'd be better to just swap manually, or use std::swap, and keep your code readable, than to do fancy tricks under the false assumption that it'll make your code faster.

To the OP, have you tried dumping your image data out as a RAW file once you've loaded it, and viewing it in an image program? Load in a 24bit TGA, and dump the contents of _image_data to a file with the extension .RAW. If the file looks fine in a program capable of using RAW images, then it means your texture is loading fine, but the code that displays it isn't.

I used this trick when testing my TGA loader, and it was a big help.

Share this post


Link to post
Share on other sites
Then there's no point in using it over manually swapping or using std::swap, all it does is make your code harder to read and debug, for someone unfamiliar with the XOR trick.

If you find that the swapping is a bottleneck, then by all means try the xor trick, but otherwise there's no point.

Share this post


Link to post
Share on other sites
its not a trick o.O its basic boolean algebra.
How is it that much harder to read?

its just a few ^='s, thats like saying
i[0] += i[2] += i[0] += i[2]; is harder to read.

in the end its a matter of preference.

I only suggested it in this case because its very useful for byte swaping.

but you can use the temporary variable method.

Just stick with what you're comfortable with.

Share this post


Link to post
Share on other sites
Nothing in the c++ standard prevents a std library provider from providing a template specialization for std::swap on unsigned chars.

I'd be very suprised if the XOR trick outperforms std::swap on any decent optimizing compiler.

Share this post


Link to post
Share on other sites
XOR won't outperform std::swap on many cases which is merly a templated version of the temporary variable solution.

Like if it were doubles then XOR would be slower. But if using ints or chars XOR tends to be slightly faster.

I don't really know how much further swap can be optimized.

Share this post


Link to post
Share on other sites
Quote:
Original post by anonuser
XOR won't outperform std::swap on many cases which is merly a templated version of the temporary variable solution.

Like if it were doubles then XOR would be slower. But if using ints or chars XOR tends to be slightly faster.

I don't really know how much further swap can be optimized.


std::swap is a template function. template functions can be specialised.

In this case that means that for a generic type it uses assignments/copy constructors and a temporary, for certain types it calls that types swap funciton (like std::vector) and for other types it may be specialised as the library writer sees fit. You won't be able to beat a std::swap on chars in C++ if the writers of the std library implementation you use were competent and you use an optimizing compiler.

The XOR trick used to provide a small gain, back when compilers didn't perform optimizations (or didn't do as many as they do today). Now it's redundant.

Now, back to the original question:

If the image appears in the correct proportions and isn't skewed, then you're problem is most likely in pixel order. Either swap the texture mode from GL_RGB to GL_BGR_EXT (or the alpha version if you need 32bit) or swap the red and blue byte order in the data you read.

Share this post


Link to post
Share on other sites
Actually, on modern superscalar architectures and in particular on architectures with a large number of registers and/or rename registers, the XOR method of swapping can be less efficient than the temporary register version since the data flow of the XOR version creates a dependency graph that interferes with the extraction of instruction level parallelism.

Share this post


Link to post
Share on other sites
Quote:
Original post by anonuser
its not a trick o.O its basic boolean algebra.
How is it that much harder to read?


When I say more difficult to read, I mean that it is more difficult to see what it does at a glance, not that it's more difficult to read the characters.

If you're not familiar with the XOR trick, then you'd have to stop, and work out what it does on paper, or with a caculator.

Since std::swap does the same thing, and it's more obvious what std::swap does, I'd argue that you'd be better off using std::swap.

Just my personal opinion.

Share this post


Link to post
Share on other sites
i got the code work with this:


for( GLuint i = 0; i < ( int )image_size; i += bytes_per_pixel )
{
// stores the blue color bit
GLuint temp = _image_data[ i + 1 ];
// switches the blue color bit with the red color bit
_image_data[ i + 1 ] = _image_data[ i + 2 ];
// put the sotred blue color bits in the thrid spot where it should be
_image_data[ i + 2 ] = temp;
}

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this