0

I've answered my question below. Thanks to gavinb for his assistance.

Question: I'm trying to replicate a texture just for a learning experience. I'm using 24bit TGA image and i'm loading it in with SDL. I've created my own structure

struct Colour24
{
    unsigned char r;
    unsigned char g;
    unsigned char b;
}

unsigned char* texture = new unsigned char[width * height * 3];

    for (int y = 0; y < height; y++)
    {
        for (int x = 0; x < width; x++)
        {
            int index = (y * width) + x;

            Colour24 colour = getColourAt(x, y);

            newTexture[index] = colour.r;
            newTexture[index + 1] = colour.g;
            newTexture[index + 2] = colour.b;
        }
    }

Colour24 getColour(int x, int y)
{
    Uint32 pixel;
    Uint8 red, green, blue;
    Colour24 rgb;

    pixel = getPixel(m_surface, x, y);  // using the SDL get pixel function

    SDL_GetRGB(pixel, m_surface->format, &red, &green, &blue);

    rgb.r = red;
    rgb.b = blue;
    rgb.g = green;

    return rgb;
}

OLD: I'm planning on sending this into an OpenGL texture and it does work when i directly send the surface->pixels. But i'd like to create the data array myself. Help is greatly appreciated!

EDIT: Sorry i didnt explain myself well enough. I'm trying to manually recreate a texture from an SDL_Surface that contains a 24bit TGA image then i'll give it to opengl to create the texture. I've tested my opengl code with surface->pixels and it works fine. I'm just trying to work out where i've gone wrong loading it in.

rocklobster
  • 609
  • 2
  • 10
  • 23
  • What's the problem exactly? Compiler error? Doesn't do what you expect? At least your code is incomplete as it contains code at the same level of a function definition. – bjhend Apr 20 '12 at 10:40
  • I'm able to load this texture into opengl but it is not correct. I get a lot of red, green and blue cross hatching happening. – rocklobster Apr 20 '12 at 10:48

3 Answers3

2

The unsigned char is almost certainly equivalent to uint8_t unless you are using some incredibly esoteric architecture.

But there are a few problems in the code, though it's not clear exactly what you're trying to do or what the problem is.

First, you've transposed the x,y coordinates when calling getColourAt(). You are using i to index the height and j to index the width, so you should be using j,i thus:

Colour24 colour = getColourAt(j, i);

Perhaps it would be simpler to stick with x,y naming.

Second, the indexing into the texture buffer is wrong. It should be:

texture[i*width+j+0] = colour.red;
texture[i*width+j+1] = colour.green;
texture[i*width+j+2] = colour.blue;

since you will have i complete rows of width bytes, and j bytes on the incomplete row.

Also, you are missing the pixel format in the SDL_GetRGB call, so this won't even compile.

Finally, your getColour() function won't work correctly; it will always return the value of the pixel at 0,0 as you aren't offsetting into the image by x,y. You can use the same technique as above for this. Just be careful with your indexing, as if you're using a UInt32 with SDL that would imply an RGBA (32-bit) pixel format, but you are using RGB (24-bit).

gavinb
  • 19,278
  • 3
  • 45
  • 60
  • Testing this out now. I also edited my original post for clarity. – rocklobster Apr 20 '12 at 10:54
  • Ok i've updated my original post with new code that i'm using. I've got different results, still weird ones though. – rocklobster Apr 20 '12 at 11:01
  • 1
    You still need to use width rather than height when multiplying by the y value indexing into the texture buffer. Also, since pixel is a UInt32, you will fall into the trap I mentioned above where you treat the buffer as 32bpp rather than 24bpp. Have a look at http://www.libsdl.org/cgi/docwiki.cgi/Introduction_to_SDL_Video#getpixel for a complete solution which should work with all pixel formats. – gavinb Apr 20 '12 at 11:08
  • Thanks, i'll check that out and see if i can get it to work for 24bit. – rocklobster Apr 20 '12 at 11:13
  • 1
    That getpixel() already does; the case of 3 for the bytes-per-pixel attribute of the pixel format does it for you. – gavinb Apr 20 '12 at 11:24
  • Hi gavinb, i've almost got it working. I can see the actual texture now which is great, the problem i'm having now is that its only 1/4 the size of the whole geometry and the rest of the texture is empty. I had to change x < width * 3 to just x < width because i was getting an access violation error in the SDL get pixel function. – rocklobster Apr 20 '12 at 11:45
  • Actually i think it was only working because it was a grey/black ish texture. I tried one with colour and i am getting the detail of the textures but the colours are all still greyscale and the size of the image is very wrong. – rocklobster Apr 20 '12 at 13:25
  • Your texture is a byte array (which means you have 3 bytes per pixel), but you are indexing into this array by counting pixels. (Also you call it `texture` in the declaration and `newTexture` in the loop.) If you're addressing bytes, you need to multiply your index by the bytes per pixel. Also, your x coordinate should not be skipping by 3 as you are using this as the pixel coord for getColourAt(). The bpp change will take care of this for you. – gavinb Apr 22 '12 at 02:27
1

Ended up writing my own TGA loader and then found out i needed to multiply the height by bytes per pixel. Still not 100% sure why (feel free to comment) but it works now.

Code:

TGAReader* reader = new TGAReader();
reader->readSurface("MyTexture.tga");

int height = reader->height;
int width = reader->width;
int bpp = reader->bpp;

unsigned char* texture = new unsigned char[width * height * bpp];

for (int y = 0; y < height * bpp; y++)
{
    for (int x = 0; x < width; x += 3)
    {
        int index = (y * width) + x;

        Colour24 colour = reader->getColourAt(x, y);

        newTexture[index] = colour.r;
        newTexture[index + 1] = colour.g;
        newTexture[index + 2] = colour.b;
    }
}
// create OpenGL texture
unsigned int = createTextureFromData(newTexture, width, height, bpp);

// reading tga data
Colour24 TGAReader::getColourAt(int x, int y)
{
    Colour24 rgb;

    rgb.r = m_data[((y * width) + x)];
    rgb.g = m_data[((y * width) + x) + 1];
    rgb.b = m_data[((y * width) + x) + 2];

    return rgb;
}
rocklobster
  • 609
  • 2
  • 10
  • 23
0

The size of Uint8 depends on its declaration. On the other hand the size of unsigned char is also not specified to exactly 8 bit in the C++ standard although it's 8 bit on many platforms.

It would be better to use Uint8 directly in your Colour24 to avoid conversion errors.

bjhend
  • 1,538
  • 11
  • 25