Advertisement

C++ & SDL2 - Big Graphics Bug - Any ideas welcome

Started by March 31, 2022 09:01 AM
21 comments, last by Nick72c 2 years, 7 months ago

JoeJ said:

Nick72c said:
…but these are all new concepts to me, so I will see if I can find a way for identical enemies objects to share surface/texture data (although I'm not immediately sure I understand how to achieve this).

Even so, do you really think improving the efficiencies in this manner would have any effect of the missing sprites? How do you see the two issues being linked?

There may be no link, but while improving something, chances are you find a bug while doing so.
About efficiency, there are multiple points why this is bad (eventually, idk what applies to your case):
* Loading the same texture many times for for many sprites wastes memory.
* Switching the current texture to another texture (which is actually the same), can cause redundant state switches on GPU, which can prevent it from keeping all its cores busy.
* Even using a string to identify something is bad, because a string has variable length, so it needs to be allocated from memory which is slow. Copying the string to another string requires a loop, which also is slower than identifying your textures with simple number, e.g. an index to an array of all your textures. Numbers have constant size, copying them is a single instruction. There also is no need to resolve indirection from the string object to the actual string data, eventually causing a cache miss.

So, to fix that, you could have a std::vector (or array) of unique texture objects, where each object also has a string of it's file path.
While creating the sprites, you could iterate this vector until you find the given path string, then only store the found index in the sprite.
You pay the price of the search only once at launch or level load, so no problem. After that it's easy to get the texture for any given sprite using this index.

But this would break if you delete one of the textures. Because it would invalidate all indices to textures after the deleted one.
Thus, many people use unique IDs instead indices, with some mechanism to find some object from a given ID. This makes things easier eventually, but has a cost of resolving indirections form IDs to actual pointers, usually causing cache misses.
So this becomes an advanced topic, e.g. when designing Entity Component Systems.

For now i'd just use indices to identify textures and other things.
It's not critical as long as you just draw some two digit numbers of sprites, but you better get rid of bad habits early.
Often the software turns out slow, but no big bottleneck shows up. So you don't know what to optimize to improve it.
To prevent this situation, you try to write optimized code from the start and all the time, which rarely is real extra work.

Thank you @joej

I'll take all of that under advisement. I'll implement what I can and research the topics I don't fully understand.

I believe I've now fixed the installer issue by switching GitHub from hosting a Setup.exe file to hosting a Parallax-Zoom_Installer.msi file,

I also added four new unique enemy sprites (simple boxes - japan, jamaica, swiss and envelope) and the results were truly Bizarre - but it's 1am here in Malaysia so it will need to wait until tomorrow.

@joej @fleabay

I'm not worthy! - but I'm very grateful ?

You guys are on it!

I stripped the surface and texture calls out of the enemies class (dumping them into main temporarily), so that all Enemy::Draw() calls use a single texture created only once, and bang all the draw bugs were gone in an instant.

Thank you ?

Just for fun I threw out a draw call for every possible enemy position at once - the game ground to an almost halt (I'll go back over your advise on optimizing the code removing strings from the Enemy Class objects, etc..) but Enemy::Draw() worked perfectly!!

I'll go back and tidy this all up - but it works great - Thank you!

Advertisement

Nick72c said:
Just for fun I threw out a draw call for every possible enemy position at once - the game ground to an almost halt

I think it might be because of the console debug output for each enemy. Also, not sure what you used but \n is much faster than endl. endl includes a stream flush each time you use it.

🙂🙂🙂🙂🙂<←The tone posse, ready for action.

fleabay said:

Nick72c said:
Just for fun I threw out a draw call for every possible enemy position at once - the game ground to an almost halt

I think it might be because of the console debug output for each enemy. Also, not sure what you used but \n is much faster than endl. endl includes a stream flush each time you use it.

Blimey! Correct again @fleabay - I took out the console commands and the lag completely disappeared, even with a screen full of enemies - Thanks again ?

@joej @fleabay

I went back and created a Class for the enemy surfaces / textures in enemyTextures.h, class EnemyTextures, with objects of enemy_image, so that I could use multiple enemy textures, but only load them once per texture as advised.

Somehow between creating the six enemy_image elements of EnemyTextures from within loadlevel1() [within levelload.cpp] only 2 out of the six elements hold the correct/valid texture?

enemy_image[4] and [5] are correct, but [3] strangely maps to [5], and [0], [1] and [2] don't appear to hold a valid textures?

Any input on where I might have gone wrong would be appreciated.

https://nickchantrell.github.io/Parrallax-Zoom-SDL2/​

I'm new to using the debugger but I believe I can also see that enemyimage[5] is assigned the same memory address as enemyimage[3] which invalidates element 3 and copies element 5 over the top of it. Why would the compiler do this when I'm trying to store SDL_Texture ptr's in this Class?

Nick72c said:
I went back and created a Class for the enemy surfaces / textures in enemyTextures.h,

Ugh - textures are not related to ‘enemies’. You probably need textures for some other things too, so you have chosen a bad name?

You want to have reusable code with minimal dependencies. Usually you make a new class ‘Texture’, in it's own file, and the class functionality should cover only what's needed to deal with a texture.
The advantage is, in your next project, you can just include this file again and you already have support for textures.

But that just said in general. Maybe your texture object has just a pointer and no functionality at all, so making an extra file isn't worth it.

Nick72c said:
I'm new to using the debugger but I believe I can also see that enemyimage[5] is assigned the same memory address as enemyimage[3] which invalidates element 3 and copies element 5 over the top of it. Why would the compiler do this when I'm trying to store SDL_Texture ptr's in this Class?

You can use the debugger to check if what you think really happens:

Add a breakpoint to line 56, where you start to fill the vector. (Just click on left border to add it)
Then run in debug mode, the debugger will stop at this line.
Then, while stepping line by line, you see what happens at texture [3].
When you get to [5], you can see if it changes [3] as well, like you assume.

It does not make sense, and i have no idea what could be wrong, but the point is: You can use the debugger to follow execution in detail, and what you find out may give you another idea of what to check next.

Advertisement

That's a fair criticism @JoeJ but as you guessed the EnemyTextures class has only one function, to hold the SDL Texture ptr's for my NPC's (enemies), so it would only take two minutes to reproduce for a future project - but I do understand your point.

As for the enemy_image[3] being overwritten as enemy_image[5] is given the same memory address, I think you can see that clearly in the screenshot I posted above, but even without this, I can see it happening as any elements I assign to ‘3’ on the level1[20][76] map initialisation array in levelload.cpp are printed in to the screen with whichever texture is held by enemy_image[5].

As I mentioned this isn't the only issue as elements 0, 1 and 2, simply refuse to work as SLD_Template ptr's - so I'll keep trying to find a solution. Thanks for your feedback.

Ok. I've finally worked through this issue.

Each time I add a new element to vector<EnemyTextures>enemy.image - the EnemyTextures class ~destructor is called [enemy.image.size() times]

This is without ever getting out of scope - so I've got no idea why the destructor is being called, but to demonstrate this clearly I moved the enemy.image.emplace_back calls into main(), and this is the result I got on the forth call down the list (note the element times destructor calls!?):

There is nothing in my code that would justify these multiple destructor calls - I'm really starting to think my Visual Studio Compiler is bugged?

Nick72c said:
Each time I add a new element to vectorenemy.image - the EnemyTextures class ~destructor is called [enemy.image.size() times] This is without ever getting out of scope - so I've got no idea why the destructor is being called

Oh my - we should have thought about this…

The reason is simple (but easy to forget):
Each time you add a new element to a std::vector, it may need to reallocate to have space for one more element.
Reallocation means: Allocate new memory, then either copy from the old to new memory and deconstruct old elements, or move elements.

For you it's obviously the former, which also is inefficient. To make moving work, you'll need to make the elements class supporting move semantics.
That's already some advanced C++ details. This post should lead to more details about it i guess: https://stackoverflow.com/questions/41864544/stdvector-calls-contained-objects-destructor-when-reallocating-no-matter-what

Another example of this causing bugs:

You have a pointer or reference to one of the elements of a std::vector (other other containers).
You add one element.
You work with the pointer or reference, which may be now invalid, causing a crash.

This is very bad, because it can happen rarely and will be hard to find eventually.
Be sure to understand and check your code for other potential cases of this issue.

This topic is closed to new replies.

Advertisement