Quick question - problems declaring an SDL_Texture container

Started by
8 comments, last by Servant of the Lord 10 years, 4 months ago

I want to define a class with a member map container that maps strings to SDL_Textures. The purpose is to hold loaded textures for reusage.


class A
{
   map<std::string, SDL_Texture> textures;
};

When I try this, I get this error:


error: 'std::pair<_T1, _T2>::second' has incomplete type

How do I enforce the definition of SDL_Texture before the declaration of the textures map, to stop this error?

Advertisement

You probably need to #include the proper SDL header than includes SDL_Texture before Class A is defined. It seems like SDL_Texture is probably just getting pre-declared before that line of code, instead of having the full definition.

I don't use the newest version of SDL, so maybe my knowledge is outdated... doesn't SDL pass around pointers to SDL_Texture, and not pass by value? In that case, you need to have SDL_Texture* as a pointer in the map, and you don't need to include anything else.

Yes, SDL uses pointers to SDL_Textures. I tried to make a map of strings to SDL_Textures pointers instead. The problem is solved...temporarily. When a member function of that class tries to access/create/assign/whatever and so dereferences one of the pointers on the map, the problem pops up again - error: 'std::pair<_T1, _T2>::second' has incomplete type...

So instead of getting an error on the map template instantiation, I get an error when using the map...

May I see the whole header file? I'll need more info that the header file could give me.

graphics.h


class Graphics
{
public:
     void renderBMP(std::string path, int posX, int posY);

private:
    // the textures container. The strings will be the file paths
    map<string, SDL_Texture*> textures;
    SDL_Texture *textureToLoad; // temporary texture holder for the loading function. The workaround.
};

graphics.cpp


/*  Search for this BMP's path on the textures container.
*   If it doesn't exist, load it to the container.
*   If it already exists, retrieve the texture from the container. */

void Graphics::renderBMP(std::string path, int posX, int posY)
{
    if (textures.find(path) == textures.end()) // If this texture isn't loaded yet
    {
        SDL_Surface *tempSurface = SDL_LoadBMP(path.c_str());
        textureToLoad = SDL_CreateTextureFromSurface(renderer, tempSurface);
        SDL_FreeSurface(tempSurface);
        textures[path] = textureToLoad;
    }
    else
    {
        textureToLoad = textures[path];
    }

    // the rest doesn't matter
}

This is how I have it, after a little workaround.

I was trying to avoid the need of a class member (in this case, textureToLoad) to hold the new SDL_Texture after the renderBMP() function has gone out of scope.

What I was trying was this:


void Graphics::renderBMP(std::string path, int posX, int posY)
{
    if (textures.find(path) == textures.end()) // If this texture isn't loaded yet
    {
        SDL_Surface *tempSurface = SDL_LoadBMP(path.c_str());
        textures[path] = new SDL_Texture(SDL_CreateTextureFromSurface(renderer, tempSurface));
        SDL_FreeSurface(tempSurface);
    }
// blablabla

I was getting the error again this way.

In the meantime I found out that SDL isn't prepared to be used in such a way that means creating SDL_Textures directly (without the pointer interface). Quoting a site I found http://stackoverflow.com/questions/18897076/sdl-texture-incomplete-type):

SDL_Texture is an opaque type by design (that is, designed to only be operated on by SDL2 internally), you can "pass it around" as a pointer, but you can't access the internals (or create a SDL_Texture yourself, for example by doing a malloc, because you don't know the size of the structure). If you stick to SDL_Texture *blah;

pointers and pass them around to the SDL2 functions you should be fine.

This I why I decided to use the workaround.

Okay, have SDL_Texture as a pointer in the map.

Then, change this line:


textures[path] = new SDL_Texture(SDL_CreateTextureFromSurface(renderer, tempSurface));

.

...and make it like this:


textures[path] = SDL_CreateTextureFromSurface(renderer, tempSurface);

Don't do the 'new SDL_Texture' because you don't have access to SDL_Texture. and because SDL_CreateTextureFromSurface() is already returning a new texture for you.

In C++, we have a saying, "Always free what you malloc, always delete what you new" - the point is, don't mix and match different pairs of allocation and deallocation functions.

Here, however, SDL is providing it's own allocation and de-allocation functions... so you never want to new or delete or free or malloc, but instead, with SDL, you want to use the functions SDL provides (which might call free and malloc internally, but that's SDL's business what it does internally).

'SDL_CreateTextureFromSurface' is the 'new' already. 'SDL_DestroyTexture' is the 'delete'. Likewise with SDL_CreateSurface (called by SDL_LoadBMP), or SDL_FreeSurface. Mixing and matching news and deletes with SDL's own functions can mess things up. smile.png

[Edit:] Whoops, slight misunderstanding on my part - it looks like you already solved the problem. Is it working now for you, or is there still a problem?

Oh, so with your code suggestion the SDL_Texture won't go out of scope? If so, it's piece of cake.

Yeah, it's working now, but getting rid of a member that only gets used on one member function would be a plus smile.png

Dynamically allocated memory doesn't go 'out of scope'; the pointers themselves do, but the memory they are pointed to doesn't if it was allocated dynamically - that's the power of dynamic memory.

What class member would you like to get rid of?

The textureToLoad smile.png

Easy enough to make it local:


void Graphics::renderBMP(std::string path, int posX, int posY)
{
    SDL_Texture *textureToLoad = nullptr; //Or 'NULL' if you don't use 'nullptr'
    
    if (textures.find(path) == textures.end()) // If this texture isn't loaded yet
    {
        SDL_Surface *tempSurface = SDL_LoadBMP(path.c_str());
        textureToLoad = SDL_CreateTextureFromSurface(renderer, tempSurface);
        SDL_FreeSurface(tempSurface);
        textures[path] = textureToLoad;
    }
    else
    {
        textureToLoad = textures[path];
    }
 
    // the rest doesn't matter
}

It can then be removed from the class as a member.

This topic is closed to new replies.

Advertisement