1

I've got a lot of code that's driving me really crazy right now. I'm working with OpenGL, building a GUI framework which utilizes several different types of objects. I have Image classes which load *.png files and store image information in the form of a GLuint texture reference. I have Panel and Button classes with pointers to the image classes they should be displaying. I have a Hud class with std::vectors of Panel and Button pointers. Finally, I have an Engine class that contains one Hud class, all my Button and Panel classes, and Image pointers. When the constructor is run, each of the Image pointers is initialized using:

imgMy = new Image;

Once all the images have been initialized, I run my load functions:

imgMy->loadImage("imgMy.png");

Of course, I delete the Images when I close the program.

My problem is that some of the images are getting "crossed." I have about thirty images right now, and a couple of the buttons are apparently pointing to the wrong images. I have checked my code thoroughly, and it appears to be solid. I believe this is a memory bug since the buttons which display the incorrect images are inconsistent. Sometimes they display the correct images, sometimes different buttons are displaying the wrong images. I wish I could show my code here, but it's pretty massive.

The reason I'm using Image pointers in my Engine class instead of actual Image objects is that I'm afraid of the Buttons pointing to invalid memory if the Engine class is resized, or its memory rearranged. I suspect there's a much better approach to what I'm trying to accomplish, and I'd appreciate any advice along those lines.

dsnettleton
  • 949
  • 2
  • 9
  • 9
  • 4
    Perhaps you could use some snippets of your code in a small exmaple that repeats the problem and post that. That exercise in itself might help you solve the problem =) Just a thought. – Josh Darnell Oct 26 '11 at 14:36
  • 2
    Describing what your code does provides little basis for diagnosing bugs. You're describing what you think the code should do, but it obvious *doesn't* do what you think it does or want it to do. As far as your fear about the Engine class being resized or its memory rearranged, at least when I'm programming, I make mistakes a lot more often than the compiler does. I'd just use objects and only switch to pointers when/if truly necessary. – Jerry Coffin Oct 26 '11 at 14:38
  • 1
    Too little information, could be caused by many things. – vz0 Oct 26 '11 at 14:38

4 Answers4

2

Use a debugger that lets you put a watchpoint on the relevant imgMys, and then the debugger will tell you where they're being modified. That is probably the easiest way to track it down.

You may also want to try valgrind, but this doesn't sound like the type of problem valgrind will find.

derobert
  • 49,731
  • 15
  • 94
  • 124
2

Firstly, you should not use two-phase initialization without a really good reason. This is not a really good reason. Pass the filename in the constructor. Also, always use smart pointers.

You could simply use const to enforce it.

class Button {
    const std::unique_ptr<Image> img;
public:
    Button(std::string filename)
        : img(new Image(filename)) {}
};

Secondly, I don't quite grok your overriding architecture, as you don't describe it in any real detail, but I'm unsure of the need of new here.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • Is there a reason for the by-value `filename` argument or is it a typo? – Christian Rau Oct 26 '11 at 14:57
  • @Christian: Meh? The compiler will likely do whatever it wants with it anyway. – Puppy Oct 26 '11 at 15:34
  • I know compilers are smart, but we shouldn't give us dumber than we are. I mean, I'm not talking about using bitshifts for integer multiplication, but passing heavy objects as const references, which is a standard idiom that shouldn't just suffer blind reliance on compiler smartness. I know there are cases where byvalue would actually enable more optimized code for a smart compiler **and wouldn't do any harm for a not so smart compiler**. If this is one of those cases and I can't see it, an explanation would be nice. Othwerwise I wouldn't encourage over-dumbification, this is not Java. – Christian Rau Oct 26 '11 at 15:42
  • @Christian: One `std::string` in a class that's going to be initialized once and held for the lifetime of the app is not a heavy object. Taking a reference here is a micro-optimization. – Puppy Oct 26 '11 at 15:51
  • It is not a micro-optimization (at least in the negative sense of the word) if it doesn't cost you any effort (no, I don't consider 7 letters effort) and doesn't obfuscate the code or its intention (which I neither think a const reference does). But Ok, this is maybe a bit subjective and you really think these rules violated by a const reference. – Christian Rau Oct 26 '11 at 16:09
  • Nah, it's more about "Anyone who does anything more than copy-paste the code would probably put it in themselves." It's not my job to optimize other people's code. – Puppy Oct 26 '11 at 17:00
0

Sounds like memory corruption. You should consider using some memory debugger like valgrind or some other alternative. If you have any issues using pointers, those tools will help you track them down.

Community
  • 1
  • 1
vz0
  • 32,345
  • 7
  • 44
  • 77
0

I suspect that you're keeping a list of button references and a list of image references somehow and that they are not always created in the same order, hence the cross over images.

Strictly speaking you should create your a button and it's image in order, assigning the image file at creation time.

ChrisBD
  • 9,104
  • 3
  • 22
  • 35