Jump to content

  • Log In with Google      Sign In   
  • Create Account

Banner advertising on our site currently available from just $5!


1. Learn about the promo. 2. Sign up for GDNet+. 3. Set up your advert!


Copying pixel data


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
5 replies to this topic

#1 CirdanValen   Members   -  Reputation: 251

Like
0Likes
Like

Posted 12 July 2012 - 12:25 AM

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).

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;
}


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

Edited by CirdanValen, 12 July 2012 - 12:35 AM.


Sponsor:

#2 Ashaman73   Crossbones+   -  Reputation: 11471

Like
1Likes
Like

Posted 12 July 2012 - 12:45 AM

This is wrong:
[source lang="cpp"]// Copy the entire row of pixelsmemcpy(&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 pixelsmemcpy(&newImage[(y-row)*w], &pixels[index], w*4);[/source]

Ashaman

 

Gnoblins: Website - Facebook - Twitter - Youtube - Steam Greenlit - IndieDB - Gamedev Log


#3 Bacterius   Crossbones+   -  Reputation: 11375

Like
3Likes
Like

Posted 12 July 2012 - 12:47 AM

&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.

Edited by Bacterius, 12 July 2012 - 05:39 AM.

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


#4 Ashaman73   Crossbones+   -  Reputation: 11471

Like
3Likes
Like

Posted 12 July 2012 - 02:32 AM

&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 pixelsmemcpy(&newImage[(y-row)*w *4], &pixels[index *4], w*4);[/source]

Ashaman

 

Gnoblins: Website - Facebook - Twitter - Youtube - Steam Greenlit - IndieDB - Gamedev Log


#5 CirdanValen   Members   -  Reputation: 251

Like
0Likes
Like

Posted 12 July 2012 - 05:11 PM

Thanks for the help guys. This is what happens when I try to program when tired :P

#6 Adachi   Members   -  Reputation: 158

Like
0Likes
Like

Posted 15 July 2012 - 09:50 AM

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.




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS