SFML community forums
Help => Graphics => Topic started by: Ashenwraith on May 01, 2010, 09:42:35 pm
-
I don't have much pointer experience so some help would be great.
I'm using sf::Uint8 pointers to store colors but they appear to be corrupted by my other pointers that I need (for image processing).
Here is a simplified example:
#include <SFML/Graphics.hpp>
#include <iostream>
void Color_Sort(sf::Uint8* color_ptr)
{
//wrong number(99)--How do I keep this as 5?
std::cout<<"color:"<<static_cast <int>(color_ptr[0])<<'\n';
}
int main()
{
sf::Uint8 *color_ptr=new sf::Uint8();
sf::Uint8 *num;
num[0]=5;
color_ptr=num;
//correct number:
std::cout<<"color:"<<static_cast <int>(color_ptr[0])<<'\n';
sf::Uint8 *color_ptr2=new sf::Uint8();
sf::Uint8 *num2;
num2[0]=99;
color_ptr2=num2;
Color_Sort(color_ptr);
delete[] color_ptr2;
delete[] color_ptr;
return 0;
}
-
sf::Uint8 *num;
num[0]=5;Dereferencing an uninitialized pointer leads to undefined behaviour.
sf::Uint8 *color_ptr=new sf::Uint8();
color_ptr=num;Here you create a memory leak. color_ptr is overwritten, you have no chance to get the dynamically allocated memory back.
delete[] color_ptr2;
delete[] color_ptr; In this code, you apply the delete[] operator to two memory areas that haven't been allocated by the new[] operator, which also evokes undefined behaviour.
The understanding of pointers and memory management is essential in C++. I recommend you to read this tutorial (http://www.cplusplus.com/doc/tutorial/pointers/), or even better, the respective chapter in a good book.
-
sf::Uint8 *num;
num[0]=5;Dereferencing an uninitialized pointer leads to undefined behaviour.
What's the difference between that and this (from the tutorial) which is "perfectly valid":
int numbers[5];
int * p;
p = numbers;
-
Your code
num[0] dereferences the pointer num. Using the index operator [], it is equivalent to the expression
*(num + 0)
In contrast,
pdoes not dereference p. It just makes the pointer (and not the object behind it) a target of the assignment.
-
Okay, so to access the specific Uint8 value within I have to break it apart like this?
sf::Uint8 num[3];
num[0]=5;
Or is there a shortcut to do something like:
sf::Uint8 *num;
num[[0]]=5;
-
Your problem is that you use pointers that don't actually point to anything valid. It's like sending a mail to someone before knowing its address.
You should really take some time to get a better understanding of how pointers work. We could try to explain it to you on this forum, but honestly it would be more efficient with a good book or tutorial ;)
-
I understand what you're saying (and I have looked at many tutorials), but maybe I wasn't clear before.
My problem is specific to returning a pointer AND using new.
For example:
sf::Uint8* Get_Color()
{
sf::Uint8 color[3];
color[0]=255;
return color;
}
int main()
{
sf::Uint8* color_ptr2=new sf::Uint8();
sf::Uint8* num2;
num2=Get_Color();
color_ptr2=num2;
return 0;
}
I understand what is happening and why, but I don't know how to return a pointer into a new memory allocation.
This is important because the pointers will not work within a loop's scope.
Is this even possible--do I have to find another solution? I've been searching for a while, but the keywords are very common.
-
You should rather tell us what you're trying to do (I mean, your high level goal), so that we can find a clean and good solution. I'm sure that it won't involve such pointer manipulations.
-
I don't know what you exactly want to achieve. Maybe you should rather tell us what you want instead of how.
Your current code contains two mistakes (pointee goes out of scope, memory leak). You can't return C style arrays. You can return pointers to dynamically allocated memory, but this is dangerous, especially at complex code. Instead, you can return wrapped arrays (using std::tr1::array).
-
Okay, I wrote my image processor and it works great.
I used pointers of sf::Color to store palettes, etc.
Then I realized if I went down to a lower level and instead used sf::Uint8 I could do more stuff.
Okay so now I have converted all my code to run on pointers of sf::Uint8 instead.
The problem is the pointers are now going out of scope/not being accurate. When I originally wrote my code I only had a few pointers.
Now I have quite a bit and I need to protect the pointer data returned from functions.
-
The only memory that you need to allocate is the image itself, after that you just need to get pointers to pixels and access their components.
For example
sf::Uint8* pixel(sf::Uint8* image, std::size_t width, std::size_t x, std::size_t y)
{
return image + (x + width * y) * 4;
}
sf::Uint8* image = new sf::Uint8[width * height * 4];
// ... fill image data ...
// Modify the pixel at (10, 20)
sf::Uint8* p = pixel(image, width, 10, 20);
p[0] = 255;
p[1] = 128;
p[2] = 0;
p[3] = 255;
// Print the pixel at (5, 8)
sf::Uint8* p = pixel(image, width, 5, 8);
std::cout << "r = " << p[0] << " "
<< "g = " << p[1] << " "
<< "b = " << p[2] << " "
<< "a = " << p[3] << std::endl;
Of course you should wrap all this stuff into lightweight (inlined) C++ wrappers, but that's how it should look like internally.
-
Alright, thanks a lot!
Didn't think of filling a blank sf::Uint8
Sorry about that.
Should have looked at the constructors.
Man it's a lot faster using sf::Uint8
Finishes an image in 1 second that used to take like 20 minutes.
-
I started testing larger images and this problem started occurring again but with my GetPixelPtr pointers.
I solved my previous problem with my custom color data because I created the data and put it into an sf::Uint8 array(which is immune to loop scope problems) and then filled my sf::Uint8 pointer image.
Maybe I need a GetPixelArray to make a pointer from?
-
Perhaps I'm mistaken, but it looks like I need to be able to do something like this and create my own pointers:
sf::Uint8* Construct_Pixels(sf::Image write_image,int w,int h,int x,int y,sf::Uint8* dest_ptr)
{
int range=(w*h)*4;
int index=(x+y*w)*4;
for(int i=0;i!=range;i+=4)
{
dest_ptr[i]=write_image.myPixels[i+index];
dest_ptr[i+1]=write_image.myPixels[i+index+1];
dest_ptr[i+2]=write_image.myPixels[i+index+2];
dest_ptr[i+3]=write_image.myPixels[i+index+3];
}
return dest_ptr;
}
-
This can be done in one line of code:
std::vector<sf::Uint8> dest(image.GetPixelsPtr(), image.GetPixelsPtr() + image.GetWidth() * image.GetHeight() * 4);
-
Hi again
Okay so I converted my base pointers to vectors using the method you provided above, but I'm getting the same problem (some sort of pointer/pixel data corruption).
So I created a small example so you can see what I'm trying to do.
Here's the original that works perfect:
sf::Uint8* img_pal=new sf::Uint8(pal_max+1);
for(int i=1;i<pal_max+2;i++)
{
img_pal[i]=pal[i-1];
}
return img_pal;
Here's a vector version that becomes corrupt later:
sf::Uint8 temp2[pal_max+1];
std::vector<sf::Uint8> pal_vct(*temp2,pal_max+1);
sf::Uint8* img_pal=temp2;
for(int i=1;i<pal_max+2;i++)
{
pal_vct[i]=pal[i-1];
img_pal[i]=pal_vct[i];
}
return img_pal;
-
sf::Uint8 temp2[pal_max+1];
What is this fixed-size array for? It doesn't appear in the first piece of code.
std::vector<sf::Uint8> pal_vct(*temp2,pal_max+1);
It's vector(size, value). So here you end up with *temp2 values equal to pal_max+1 (don't you set the maximum level of warnings, so that your compiler can tell you that?).
If you really want the equivalent of the first example -- assuming that you meant "new[...]" not "new(...)", it's very easy:
std::vector<sf::Uint8> img_pal(pal_max+1);
for(int i=1;i<pal_max+2;i++)
{
img_pal[i]=pal[i-1];
}
return img_pal;
-
Hey again, thanks for the code as usual.
Sorry for the bad example, I included the pointer to allocate for the vector because it seemed to get the same results with or without so I thought I might as well include it.
Anyways the problem wasn't the vectors\.
I lost my original code I was converting when I overwrote it with another file by accident when code blocks crashed (and I just copy and pasted an example I saw online to test).
I found some old images from my original code (I saved out every pass) and found out the passes with my new code were accurate until a certain pass that corrupted them.
This part had a lot of extra stuff that needed to be added for the conversion, so rechecked and fixed it and now it works perfect.
Thanks again.