4

I'm trying to make a simple game and when I try rendering my SDL_Texture, I get an inexplicable error. I've set up everything right, I'm able to successfully clear the screen with SDL_RenderClear, and my texture isn't null, so it should have been created properly. But when I try calling the render() function I get an error, and SDL_GetError() returns "Invalid texture".

Edit: I have now created an MCVE as requested, and I tested it to verify that it reproduces the error. It should display the image at the path "gfx/grid.bmp" in the window, but instead it gives me the error. Here is the full MCVE:

#include <SDL.h>
#include <string>
#include <iostream>
#undef main

class Texture{
private:
    SDL_Texture* texture;
    SDL_Renderer* renderer;
    int width;
    int height;
public:
    Texture(){
        texture = nullptr;
    }
    Texture (SDL_Renderer* ren, std::string path){
        texture = nullptr;
        SDL_Surface* surface = nullptr;
        surface = SDL_LoadBMP(path.c_str());
        if(surface == nullptr) return;
        renderer = ren;

        texture = SDL_CreateTextureFromSurface( renderer, surface );
        if(texture == 0) return;

        width = surface->w;
        height = surface->h;

        SDL_FreeSurface(surface);
        std::cout << "Texture '" << path << "' successfully loaded\n";
    }
    bool loaded(){
        return texture != 0;
    }
    bool render(int x = 0, int y = 0){
        SDL_Rect dest;
        dest.x = x;
        dest.y = y;
        dest.w = width;
        dest.h = height;
        return SDL_RenderCopy(renderer, texture, nullptr, &dest) == 0;
    }
    void unload(){
        if(loaded()){
            SDL_DestroyTexture(texture);
            texture = nullptr;
            renderer = nullptr;
            width = 0;
            height = 0;
        }
    }
    ~Texture(){
        unload();
    }
};

class CApp{
private:
    bool running = true;

    SDL_Window* window = nullptr;
    SDL_Renderer* renderer = nullptr;
    Texture graphic_grid;
    bool render = true;
public:
    int OnExecute();

    bool OnInit();
    void OnEvent(SDL_Event* event);
    void OnRender();
    void OnCleanup();
};

bool CApp::OnInit(){
    if(SDL_Init(SDL_INIT_EVERYTHING) < 0){
        return false;
    }
    if((window = SDL_CreateWindow("Simple Tic-Tac-Toe", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, 600, 600, SDL_WINDOW_SHOWN)) == nullptr){
        return false;
    }
    renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);
    if(renderer == nullptr){
        std::cout << "Renderer failed to load! Error: " << SDL_GetError();
        return false;
    }
    SDL_SetRenderDrawColor( renderer, 0xFF, 0xFF, 0xFF, 0xFF );

    graphic_grid = Texture(renderer, "gfx/grid.bmp");

    if(!graphic_grid.loaded()){
        std::cout << "Image failed to load! Error: " << SDL_GetError();
        return false;
    }

    std::cout << "Initialized fully\n";

    return true;
}

void CApp::OnEvent(SDL_Event* event){
    switch(event->type == SDL_QUIT){
        running = false;
    }
}

void CApp::OnRender(){
    if(!render) return;

    if(SDL_RenderClear(renderer) == 0){
        std::cout << "Successfully cleared screen\n";
    }

    if(!graphic_grid.loaded()){
        std::cout << "Texture not loaded!\n";
    }else{
        std::cout << "Texture not null!\n";
    }

    //This is the place where I get the "Invalid texture" error, everything else works fine
    if(!graphic_grid.render()){
        std::cout << "Failed to render image! Error: " << SDL_GetError() << '\n';
    }

    SDL_RenderPresent(renderer);

    render = false;
}

void CApp::OnCleanup(){
    graphic_grid.unload();

    SDL_DestroyRenderer(renderer);
    SDL_DestroyWindow(window);
    renderer = nullptr;
    window = nullptr;

    SDL_Quit();
}

int CApp::OnExecute(){
    if(OnInit() == false){
        return -1;
    }

    SDL_Event event;
    while(running){

        while(SDL_PollEvent(&event)){
            OnEvent(&event);
        }

        OnRender();
    }

    OnCleanup();

    return 0;
}

int main(){
    CApp theApp;
    return theApp.OnExecute();
}

I have searched around and can't find anything that explains what my error could be. The closest thing I could find was this question, but that error was caused by the renderer being destroyed before the texture, which isn't my case.

I really hope someone can tell me what might be causing this error, any suggestions are appreciated!

Community
  • 1
  • 1
snap64
  • 53
  • 1
  • 6
  • There's too many unknowns here, strip your code down to an [mcve](http://stackoverflow.com/help/mcve) that shows the issue and that we can simply copy, paste and compile. – user657267 Sep 09 '14 at 07:18
  • Alright, I added an MCVE to my question :) sorry I didn't do this to begin with, I'm a bit new to this. – snap64 Sep 09 '14 at 16:23

2 Answers2

1

Your problem is that you are copying your Texture in this line

graphic_grid = Texture(renderer, "gfx/grid.bmp");

Since you do not define a copy assignment operator or a move assignment, the compiler provides one for you, which simply copies the members as-is.

As soon as the temporary created by Texture(renderer, "gfx/grid.bmp"); is destroyed, graphic_grid.texture is no longer valid. You can test this by getting rid of your Texture destructor, you should find everything works as expected (of course you don't actually want to do this, it isn't a fix).

You will need to implement either a move assignment or copy assignment constructor, preferably the former if your Textures are not meant to be copied (and since you need the assignment you should also provide a move/copy constructor too, along with the other two move/copy methods as per the rule of five, or at least explicitly delete them).

Please read up on the rule of 0/3/5

Rule-of-Three becomes Rule-of-Five with C++11?

If you are using GCC the -Weffc++ flag will warn you about some parts of dodgy class design like this.

Personally I try to wrap handles and pointers into RAII types as much as possible, as long as each member is responsible for itself you usually don't have to faff around with copy and move constructors. std::unique_ptr works quite well for this, e.g.:

template<typename T, void(*destroy)(T*), typename Ptr = std::unique_ptr<T, decltype(destroy)>>
struct Handle : public Ptr
{
  Handle(T*&& p): Ptr{p, destroy}
  {
    if (!*this)
      throw std::runtime_error{SDL_GetError()};
  }
};

using Window_h  = Handle<SDL_Window, SDL_DestroyWindow>;
using Surface_h = Handle<SDL_Surface, SDL_FreeSurface>;
// And so on for other handles

Can be used as follows

Window_h window{SDL_CreateWindow(
  "Hello world", 
  SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED,
  800, 600,
 0 
)};

If you were to wrap SDL_Texture as above then your Texture class would have an implicit move constructor and move assignment operator that would just work out of the box (you would have to get rid of the renderer member however, or handle it differently, for instance make it a std::shared_ptr so it isn't destroyed along with the texture unless its reference count is 0).

There's a few other odd things about your code, such as prematurely returning from a constructor. If an object cannot be constructed you should throw an exception.

Community
  • 1
  • 1
user657267
  • 20,568
  • 5
  • 58
  • 77
  • Thank you so much! I removed the destructor like you said and it works fine :D I realize now that I don't even need the destructor as I call `unload()` at the end of the execution anyway. Thank you again! – snap64 Sep 10 '14 at 03:11
  • @snap64 Perhaps I didn't explain myself well enough: deleting the constructor only masks the problem, if you don't handle your copying / moving semantics properly it WILL come to bite you in the ass again at some point. Think about what happens if you inadvertently copy your texture again, a single call to `unload` will destroy every single copy of that particular texture, this is really bad design. – user657267 Sep 10 '14 at 03:26
  • Thank you, I will make sure to add those functions to the class :) – snap64 Sep 15 '14 at 00:13
0

I don't know which constructor you are calling to create object of your Texture class, I think there is some issue with the surface you are creating. I have tried your code with some changes and it is working fine,

void CApp::OnRender(){

    SDL_Window* pWindow;
    SDL_Renderer* render;
    SDL_Surface* surface;

    SDL_Init(SDL_INIT_VIDEO);
    // Create wndow 
    pWindow = SDL_CreateWindow( 
        "Dummy", 
        100, 
        100, 
        640, 480, 
        SDL_WINDOW_SHOWN); 
    // Create renderer 
    render = SDL_CreateRenderer(pWindow, -1, 0); 
    // Create Surface
    surface = SDL_GetWindowSurface(pWindow);

    // Creating object of class texture
    Texture graphic_grid(render, surface);

    if(SDL_RenderClear(render) == 0){
        std::cout << "Successfully cleared screen\n";
    }

    if(!graphic_grid.loaded()){
        std::cout << "Texture not loaded!\n";
    }else{
        std::cout << "Texture not null!\n";
    }
    if(!graphic_grid.render()){
        std::cout << "Failed to render image! Error: " << SDL_GetError() << '\n';
    }
    for(int i = 0; i < 9; i++){
        if(grid[i] == 0) continue;
        int x = i%3*200;
        int y = i/3*200;
        if(grid[i] == 1){
            graphic_x.render(x, y);
        }else{
            graphic_o.render(x, y);
        }
    }

    SDL_RenderPresent(render);

    render = false;
}
Michael Shaffer
  • 374
  • 2
  • 16
Hemant Gangwar
  • 2,172
  • 15
  • 27
  • Sorry for being unclear about that, I was using the other constructor to load an image from file. – snap64 Sep 09 '14 at 16:30