-1

For starters, let's consider the following abstract from a project I'm currently working on. Here's the header:

class GUI {
public:
    GUI& Initialize();
    void DrawGUI(float width, float height);

private:
    Model quad;
    Shader sh;
    Camera cam;
};

While this is the cpp file:

GUI& GUI::Initialize() {
    quad.Initialize().LoadModel("Resources/quad.obj");
    sh = Shader().Initiliaze();
    sh.models.push_back(&quad);
    return *this;
}

void GUI::DrawGUI(float width, float height) {
    quad.SetScale(width, height, 0);
    sh.RenderModels();
}

Before going straight to the problem an explanation is due. The Shader object sh stores pointers to a certain number of models in a vector vector<Model*> models. These models can be rendered using the RenderModels method. During initialization, GUI adds quad to sh.models via quad's address &quad. Then the method DrawGUI can be used to scale quad and to draw it on the screen.

The Problem:

When using DrawGUI in my main function, the scaling has no effect!

This is due to a "mysterious" behavior that I was hoping someone could unravel for me. I already know the root of the problem, but I cannot grasp what's behind it. Although the SetScale function modifies the dimensions of quad, this does not affect the rendering since (I have verified this while debugging) &quad and sh.models[0] have different values when DrawGUI is called. In particular, quad changes its address without any intervention on my part at one point between initialization and draw call. A small change fixes the problem and the scaling takes place:

void GUI::DrawGUI(float width, float height) {
    sh.models[0].SetScale(width, height, 0);
    sh.RenderModels();
}

But this does not help me to understand why the problem occurs in the first places. Things are made even less clear by the fact that, while the problem occurs when GUI is declared using automatic storage, i.e. GUI gui;, it does not appear when I allocate GUI on the heap. In fact quad does not change address keeping it the same as sh.models[0] and everything works fine.

Why the different behavior between the two styles of allocation? Is there something about automatic allocation that I'm missing? Why does quad change its address?

StrG30
  • 670
  • 1
  • 10
  • 20
  • 1
    If the address of `quad` mysteriously changes it suggests that you have new `GUI` objects being instantiated when you're not expecting it. E.g. if you `push_back` a `GUI` to a container it will create a copy of it. You haven't shown enough code to know if this is the case though, but if you put a `cout` in the `GUI` constructor you should easily be able to tell if that's what's happening. – Jonathan Potter Jun 05 '16 at 01:57
  • Have you tried using a deleted copy constructor? My guess is you have GUI getting passed as a copy to some function, or something. – Todd Christensen Jun 05 '16 at 01:58
  • @JonathanPotter I just tried the `cout` trick. I got only one print... – StrG30 Jun 05 '16 at 02:06
  • Did you add an explicit copy constructor? – Jonathan Potter Jun 05 '16 at 02:09
  • Delete copy constructor and assignment operator, and recompile the project. My bet is,you will see compilation errors. – SergeyA Jun 05 '16 at 02:19
  • @SergeyA I have deleted the copy constructor and the problem revealed itself. Basically the issue is with the line `GUI gui = GUI().Initialize();` in my main function and the fact that `Initialize` returns `*this`. Nonetheless, I'm not 100% sure of what's going on here. I have a decent understanding of references, lvalues and rvalues, but I don't get why returning `*this` and using it as I did causes the problem. Can someone give me a hint? – StrG30 Jun 05 '16 at 02:31

1 Answers1

2

With latest comments, the problem is clear now. Following line exists in OP's code:

GUI gui = GUI().Initialize();

And the most important part of Initialize is:

GUI& GUI::Initialize() {
    // ...
    return *this;
}

As a result, a copy-constructor is called of the GUI object. Absent any user-provided code, a default copy-constructor is used - and as OP also mentions pointers, those pointers are copied as part of default copy constructor. But no one is copying those objects! As a matter of fact, nobody was calling Initialize on the new object.

Solution is not clear from the given snippets, but couple of things are obvious:

  • Do not use Initialize function. Use constructor, that's what it is for.
  • Carefully think about copying your object. When in doubt, delete both copy constructor and assignment operator.
  • Do not use raw pointers for managing object, this is what smart pointers are for. Among smart pointers, your first choice should be unique_ptr.
SergeyA
  • 61,605
  • 5
  • 78
  • 137