Search code examples
c++sdlsdl-2

SDL2 - Why is a text flickering if my color is not initialized as a private variable?


I try to present text with the following code:

void Text::display(SDL_Renderer *renderer, int x, int y) {
// pos is an SDL_Rect and font is a TTF_Font
    pos.x = pos.w = x;
    pos.y = pos.h = y;

    SDL_Surface *surface = TTF_RenderText_Solid(font, text.c_str(), color);
    SDL_Texture *texture = SDL_CreateTextureFromSurface(renderer, surface);
    SDL_QueryTexture(texture, NULL, NULL, &pos.w, &pos.h);
    SDL_RenderCopy(renderer, texture, NULL, &pos);

    SDL_FreeSurface(surface);
    SDL_DestroyTexture(texture);
}

In my Text class. At first I had an uninitialized SDL_Color color in my Text::display() method which let me present a text on the screen. (renderer is passed from main + coordinates x,y). But I decided to make my SDL_Color color a private variable in the class Text instead. And what is weird to me is that as a private variable the text was flickering once I presented it, but if I set it as a public variable or placed it in the method the text was not flickering. Or if I initialized it as a private variable (SDL_Color color = {255, 255, 255}).

My question is if there was only pure luck that it worked when color was uninitialized as a public or method variable or if they are treated differently? When I initialized color in the constructor it was also flickering if color was private.

My main method:

void fpsCap(Uint32 starting_tick) {
    if ((1000 / fps) > SDL_GetTicks() - starting_tick) {
        SDL_Delay(1000 / fps - (SDL_GetTicks() - starting_tick));
    }
}

int main(int argv, char** args) {

// Create renderer, window, etc

    while (true) {
        SDL_RenderClear(renderer);
        Text *txt = new Text("hello");
        txt->display(gui->getRenderer(), 0, 0);
        SDL_RenderPresent(renderer);
    }
}

Solution

  • The value of the private member is not initialized, and so it gets random/garbage value. As you allocate new Text instance for every frame. You allocate on heap (every time in a new place), so it is sort of guaranteed to actually be garbage.

    Why it didn't flicker in other cases?

    • Public member: my guess is that you also made it static? Static variables are zero-initialized (contrary to member variables).

    • Local variable: local variables are not zeroed and are considered to contain garbage, but as they are stack-allocated, they are likely to get identical piece of garbage every single time in the given place.

    • Private member assigned in the constructor: that is unexpected. Are you sure that it was the same constructor that is actually used? And that it was the same variable? Perhaps some name shadowing prevented the value from actually landing where our should?

    Tangential:

    You leak a Text instance every loop. You should delete every object created with new, or better, avoid using new altogether. Use unique_ptr/make_unique instead, or just local variables (much better in this case).

    EDIT to answer questions about new:

    In modern C++ you almost never need to use new.

    • If you have an object that is used only during the execution of the function, the natural thing to do is to keep it directly (i.e. not through a pointer), in this case defining the variable as

      Text txt("hello");
      

      This is a similar idiom to Java's Text txt = new Text("hello");, but the result variable is an instance itself, and not a reference to it.

    • If you want to create an instance that you immediately pass as an argument of type Text or const Text&, you can create it as temporary like SomeFunction(Text("hello")) (note the lack of new, it's just the class name).
    • If you need heap allocation, smart pointers (std::unique_ptr and std::shared_ptr) created with std::make_unique and std::make_shared are strongly preferred, as they guard from many common pitfalls such as memory leaks and dangling pointers, at least as long as you keep hold of the smart pointer itself. std::shared_ptr is probably closest thing standard C++ has to Java reference, although it's not garbage collected, so if you make a cycle of them, they won't be ever freed.

      You could replace your Text *txt = new Text("hello"); with

      auto txt = std::make_unique<Text>("hello");
      

      to get the same behavior, but with the Text getting destroyed and deallocated at the end of the block.