Copying pixel data

Started by
4 comments, last by Adachi 11 years, 10 months ago
I'm trying to write some code to copy a smaller section of a bigger image to a new image for use in my game. I'm using SFML to load the image from the file. I know the format of the pixels are RGBA and are stored in a block of memory sized as width*height*4. I assume this means that each pixel is 4 bytes(1 bit per color).

[source]Texture* AssetLoader::getSubTexture(const std::string &name, int x, int y, int w, int h) {
sf::Image image;

if(!image.loadFromFile(name)) {
return NULL;
}

// Get the pointer for the image's pixels
const unsigned char *pixels = image.getPixelsPtr();

// Create new buffer for our new image
unsigned char *newImage = (unsigned char*)malloc(w * h * 4);
unsigned int imageWidth = image.getSize().x; // Pixels per row
unsigned int imageHeight = image.getSize().y; // Pixels per column

// Copy the pixels into the new image
for(unsigned int row = y; row <= y+h; row++) {

unsigned int index = x + (row*imageWidth);

// Copy the entire row of pixels
memcpy(&newImage[(row-y)*w], &pixels[index], w*96);
}

Texture* result = new Texture((const void*)newImage, w, h);
delete[] newImage;
return result;
}[/source]

This code works for my current testing situation. The image I'm testing on is 32x32. The portion of the image I'm copying with this method is (0,0,32,32).

memcpy(&newImage[row*w], &pixels[index], w*96);

With this code, I'm trying to copy an entire row of pixels into the new buffer. I would assume that, to copy an entire row of pixels, the size of memory I'm copying would be widthOfImage*4bytes. However, this only copies the fist few lines of the image and stops. When I do w*96 as in my example, the entire image gets copied. Of course, this breaks down if I try to copy a different size image.

My question is, what is happening here and how do I properly copy the pixels? I can't seem to figure out where the number 96*32 comes from that makes the image load fully.

32x32x4 = 4096
96*32 = 3072
----
1024
Advertisement
This is wrong:
[source lang="cpp"]// Copy the entire row of pixels
memcpy(&newImage[(row-y)*w], &pixels[index], w*96);[/source]
1. you have 4 bytes, not 96.
2. it is y-row not row-y, this gets negative, this is the reason you copy only the first line. Increasing 4 to 96 is a hack to copy more, but still a bug.

The correct version:
[source lang="cpp"]// Copy the entire row of pixels
memcpy(&newImage[(y-row)*w], &pixels[index], w*4);[/source]
&pixels[index]
This is where you failed. Pixels is an unsigned char array, not a pixel array, so you are accessing the index'th byte, not the index'th pixel. You need to multiply the index by four (whatever your pixel size is) to reach the correct offset (and account for row padding, but there is none for RGBA images). And yes, the 96 doesn't make sense, it should be a 4.

“If I understand the standard right it is legal and safe to do this but the resulting value could be anything.”


&pixels[index]
This is where you failed. Pixels is an unsigned char array, not a pixel array, so you are accessing the pixel'th byte, not the pixel'th pixel. You need to multiply the index by four (whatever your pixel size is) to reach the correct offset (and account for row padding, but there is none for RGBA images). And yes, the 96 doesn't make sense, it should be a 4.

Therefore the correcter version should look like this:
[source lang="cpp"]// Copy the entire row of pixels
memcpy(&newImage[(y-row)*w *4], &pixels[index *4], w*4);[/source]
Thanks for the help guys. This is what happens when I try to program when tired :P
I just wanted to add, I don't think it's a good idea to mix malloc/delete. You should use 'new' to allocate your memory.

This topic is closed to new replies.

Advertisement