sf::Sprite Rotation causes Crash

[shmup73]

Hey guys,

I'm having trouble with rotating my sprite, which does sound quite noobish^^, but i just can't figure out what's wrong ...

First i thought that something about my class hierarchy was wrong but actually even using the unwrapped format of sf::Sprite.Rotate(float fArg) or .SetRotation(float fArg) always cause my app to crash :/

Example:

Code: [Select]


//Function for loading an image from file.
sf::Image General::LoadIMG(Cpt _file)
{
    sf::Image img = sf::Image();

    if(!img.LoadFromFile(_file))
        std::cout << lend;

    delete _file;
    return img;
}

int main()
{
    RenderWindow window(
                VideoMode(1280,640,32),
                "SFML 1.6 in Qt Creator",
                Style::Close, WindowSettings(24,8,0));

    Image img = General::LoadIMG("Arrow.png");
    Sprite spr = Sprite(img);
}


And then in the main loop (of course after clearing and before displaying the window) :

Code: [Select]


window.Draw(spr);


Loading and drawing works just fine, but whenever i add spr.Rotate( e.g. 0.5f ), or spr.SetRotation( 0.5f ) to any part of the code the program crashes at runtime.

Code: [Select]


window.Clear(...);
spr.Rotate(0.5f);
window.Draw(spr);
window.Display();


= Crash

PS: I'm using SFML 1.6 cpp binding in Qt Creator.

Hopefully i'm just being blind, any suggestions would be much appreciated!

[Laurent]

We need a minimal code. For example, does the following code crashes too?

Code: [Select]

#include <SFML/Graphics.hpp>

int main()
{
    sf::Sprite sprite;
    sprite.Rotate(0.5f);

    return 0;
}

If it doesn't, then you must find which part of your code makes it crash.

[shmup73]

Ok so first of all, thanks for the fast reply

I've had another look at the code and surprisingly this didn't crash:

Code: [Select]



int main() {
Sprite spr;
spr.Rotate(0.5f);
return 0; }



Also drawing the sprite worked fine, so the problem seems to be caused by my General::LoadIMG( ) function, however i dunno why, since loading and drawing sprites created by this method works, but when it comes to rotating such a sprite it does crash

This means replacing

Code: [Select]


Image img = General::LoadIMG(file);
Sprite spr = Sprite(img);

// by

Image img;
img.LoadFromFile(file);
Sprite spr = Sprite(img);


solves the problem however I don't know what's wrong about the img loading function...

Code: [Select]


//Cpt = typedef char*

sf::Image General::LoadIMG(Cpt _file)
{
    sf::Image img;

    if(!img.LoadFromFile(_file))
        std::cout << lend;

    delete _file;
    return img;
}


As I said only loading and drawing images getting created by this method works, but rotating them not??

Thanks for helping me to find the error, but still i'd like how to fix the LoadIMG function or more specifically why rotating images loaded by this function couldn't work while drawing them does.

Thanks in advance

[Laurent]

Sorry but I still need a minimal code

Your function looks ok so I won't be able to really help you unless you provide me a complete piece of code that reproduces the problem.

What is this Cpt class? Are you sure that you must delete _file?

[shmup73]

Thanks for responding Laurent.

As I wrote as a comment in the last snippet
Cpt is a typedef for char* so its just a pointer to a
bunch of chars that's freed after being passed to the
sf::Image.LoadFromFile function as the image file name.

I'll try to make the problem it a little bit clearer

While this code runs without any problems, that means no crash and
the image is displayed correctly:

Code: [Select]


#include <SFML/Graphics.hpp>

int main()
{
    RenderWindow window(...);
    Image img = General::LoadIMG("myIMG.png");
    Sprite spr(img);
   
    //Main Loop
    window.Clear(...);
    window.Draw(spr);
    window.Display(...);

    return 0;
}


This one is going to crash although only the Rotate() call is added:

Code: [Select]


#include <SFML/Graphics.hpp>

int main()
{
    RenderWindow window(...);
    Image img = General::LoadIMG("myIMG.png");
    Sprite spr(img);
    spr.Rotate(0.5f); //Rotate the sprite.
   
    //Main Loop
    window.Clear(...);
    window.Draw(spr);
    window.Display(...);

    return 0;
}


But since doing this replacement fixes the error
(i.e. it then does show up AND rotate):

     { Image img = General::LoadIMG("myIMG.png"); }
     becomes...
     { Image img; img.LoadFromFile("myIMG.png"); }

It means that this function

Code: [Select]


sf::Image General::LoadIMG(Cpt _file)
{
    sf::Image img;

    if(!img.LoadFromFile(_file))
        std::cout << lend;

    delete _file;
    return img;
}


needs to be causing a crash whenever i call a method associated with the sprite's rotation.
But the strange thing is that - as you can see in the first snippet of this post - i'm able to render the image loaded by my wrapped function,
 while rotating doesnt work? that would mean it does only partially load the image which sounds pretty unlikely...:/

So here's a complete (as minimal as possible ) file.
I've moved the img loading function in there for maximal minimality^^

Code: [Select]


#include <SFML/Graphics.hpp>
#include <iostream>

#define lend std::endl

using namespace sf;

Image LoadIMG(char *_file)
{
    Image img;

    if(!img.LoadFromFile(_file))
        std::cout << lend;

    delete _file;
    return img;
}

int main()
{
    RenderWindow window(VideoMode(1280, 640, 32), "Caption goes here");
    Event event;

    Image img = LoadIMG("myIMG.png");
    Sprite spr(img);
    spr.Rotate(0.5f); //!Runs without this line and crashes with!

    while(window.IsOpened())
    {
        while(window.GetEvent(event))
        {
            switch(event.Type)
            {
                case event.Closed:
                window.Close();
                break;
            }
        }

        window.Clear();

        window.Draw(spr);

        window.Display();
    }

    return 0;
}


This snippet does always crash on my system and if i remove the spr.Rotate(0.5f); line it does draw the sprite without complaints <.<
I really don't know why it does only crash when rotating it, i mean wouldn't i have to crash when loading / drawing the image if the loadIMG function is the problem which it is since rotation does work when i load the image using img.LoadFromFile() :/

Any ideas how to fix the LoadIMG function?
Otherwise i'll just have to stuck with the unwrapped (still awesome img.LoadFromFile() function.

Anyways thanks alot i'm really appreciating the community and your library of course

Looking forward to any replies,
greetings shmup.

[shmup73]

Omg, removing the "delete _file;" line fixed it^^

And yet another reason why i hate pointers
Still i would really like to know why deleting the pointer after it's passed to the load image file causes a crash
 when rotating i mean is there something like streaming pixel data from the file the image is loaded from when its rotated i.e. is the app trying
 to dereference the char* representing the image's path at runtime after it's deleted?

If anyone has a clue please let me know

[Laurent]

You never allocated it with the new keyword, so you musn't deallocate it with delete. That's a very simple rule of C++ that must always be followed.

What happened here is that you freed a memory area that is probably a read-only zone reserved by the compiler for strings literals. That can mess things up a lot.

Anyway, all you have to remember is that you created an undefined behaviour by calling delete where you shouldn't, and "undefined behaviour" means that anything can happen. You don't have to understand why this made the call to Rotate crash later; you corrupted the memory and a bad thing happened, there's no logical link between the cause and the crash.

[Nexus]

You may only delete objects that have been previously allocated with new. This is not the case here. Anyway, you should try to use manual memory management (i.e. new and delete) as few as possible in user code. It is hardly ever necessary when you work with the RAII idiom.

By the way, #define lend std::endl is a really bad idea, even more because "lend" is a real word and very short. Rather think about using std::endl; if this isn't located in a header.

[Silvah]

Anyway, you should try to use manual memory management (i.e. new and delete) as few as possible in user code.

Would you care to explain what's wrong with using new?

[shmup73]

Thanks at all, Pointers a still pretty new to me since i switched over from C#, so i'm glad you gave my some tips

[Dimple]

Would you care to explain what's wrong with using new?

There's nothing wrong with using it as long as you remember to delete it (and never delete it too early) but using it is error prone.

[Nexus]

Would you care to explain what's wrong with using new?

Using new and delete with raw pointers introduces unnecessary complexity and allows much more errors. Owning pointers to objects allocated with new have no value semantics, i.e. you must overload or forbid the Big Three explicitly, you have to be very careful when storing the pointers in STL containers (std::remove_if can't be meaningfully applied, you can't copy a container). Exception safety is much harder to achieve and needs a lot of code. Memory leaks, double deletes, dangling pointers are always an issue one has to think of. With the use of RAII, life is much easier.

Code: [Select]

struct MyClass
{
    A a;
    B b;
};
vs.

Code: [Select]

class MyClass
{
     A* a;
     B* b;

     // Implement the following methods correctly even for the case
     // where the constructor of the B object throws an exception
     MyClass();
     MyClass(const MyClass& origin);
     MyClass& operator= (const MyClass& origin);
     ~MyClass();
}

[OniLinkPlus]

Anyway, you should try to use manual memory management (i.e. new and delete) as few as possible in user code.

Would you care to explain what's wrong with using new?

It's very easy to forget to call delete. Also, people almost always use it when it's not necessary at all.

[Silvah]

Okay, but you two didn't answer my question at all

I know what's wrong with raw pointers and with calling delete manually. I just don't get why new suddenly became bad. What's wrong with, for example, the following code? (there must be something wrong, because, uhm, I used new! )

Code: [Select]

scoped_ptr<Base> ptr(someCondition ? new DerivedA(foo) : new DerivedB(bar));

[Groogy]

The bad thing is that it adds more complexity to the code. Also it gives us potential errors. Sometimes you are forced to use pointers, but if you get to choose between having a pointer and not having it, the choice should be obviously clear.