Search code examples
csdlsdl-2

Not able to draw an array of squares in SDL2 in C


I'm using SDL2 for C to draw out differently coloured squares based on an array (terrain[something][something]). But when I run it, what happens is a red square gets drawn top left and then the window stops responding.

Basically what I'm doing is using a couple of for cycles to go through the array and based on the value of the given element, it draws a differently coloured square at a bit of a different position each time. I'm actually not sure if the stuff I draw gets automatically cleared when I draw the next one as I'm using the same rect structure.

int terrain_print()
{
    if (SDL_Init(SDL_INIT_EVERYTHING) != 0)
    {
        printf("Error initializing SDL: %s\n", SDL_GetError());
        return 0;
    }
    SDL_Window* wind = SDL_CreateWindow("Terrain Generator",
        SDL_WINDOWPOS_CENTERED,
        SDL_WINDOWPOS_CENTERED,
        WIDTH, HEIGHT, 0);
    if (!wind)
    {
        printf("Error creating window: %s\n", SDL_GetError());
        SDL_Quit();
        return 0;
    }
    Uint32 render_flags = SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC;
    SDL_Renderer* rend = SDL_CreateRenderer(wind, -1, render_flags);
    if (!rend)
    {
        printf("Error creating renderer: %s\n", SDL_GetError());
        SDL_DestroyWindow(wind);
        SDL_Quit();
        return 0;
    }
    bool running = true;
    float x_pos = 0, y_pos = 0;
    SDL_Rect rect = { (int)x_pos, (int)y_pos, PIXELSIZE, PIXELSIZE };
    SDL_Event event;
    while (running)
    {
        while (SDL_PollEvent(&event))
        {
            switch (event.type)
            {
                case SDL_QUIT:
                    running = false;
                    break;
            }
        }
        for (int i = ARRAYH - 1; i >= 0; i--)
        {
            for (int j = 0; j < ARRAYW; j++)
            {
                switch (terrain[i][j])
                {
                    case air:
                        SDL_SetRenderDrawColor(rend, 75, 163, 255, 255);
                    case grass:
                        SDL_SetRenderDrawColor(rend, 0, 181, 16, 255);
                    case dirt:
                        SDL_SetRenderDrawColor(rend, 74, 26, 0, 255);
                    case stone:
                        SDL_SetRenderDrawColor(rend, 85, 85, 85, 255);
                }
                SDL_SetRenderDrawColor(rend, 255, 0, 0, 255);
                SDL_RenderFillRect(rend, &rect);
                SDL_RenderPresent(rend);
                SDL_Delay(1000 / FPS);
                x_pos += PIXELSIZE;
            }
            y_pos += PIXELSIZE;
        }
    }
    SDL_DestroyRenderer(rend);
    SDL_DestroyWindow(wind);
    SDL_Quit();
    return 0;
}

Solution

  • As has already been pointed out in the comments section by someone else, there are probably break statements missing in one of your switch statements. However, even if you had these break statements, the contents of the entire switch statement would be overridden by changing the color back to red in the following line:

    SDL_SetRenderDrawColor(rend, 255, 0, 0, 255);

    Also, instead of updating x_pos and y_pos, you must update rect.x and rect.y. Otherwise, you will always be overwriting the same rectangle.

    Another issue could be that according to the official documentation on SDL_RenderPresent, it is recommended to call SDL_RenderClear before starting each new frame's drawing. The documentation states that you should not rely on the previous frame's contents still existing in the backbuffer, when you start drawing the next frame. However, you seem to be relying on this. Even if your program works on your computer without calling SDL_RenderClear, it may stop working when you change the configuration or run the program on a different platform. I assume that this warning was put in the documentation for a good reason. Therefore, you might want to change your algorithm to render everything in one frame, or, if you want maintain the appearance of a new rectangle being added in one frame, render all rectangles from the previous frames in the new frame. Alternatively, you could use SDL_SetRenderTarget to render to an SDL_Texture instead of directly to the screen, so that you can be sure that the contents never get lost.


    and then the window stops responding.

    This is not surprising. You are calling SDL_RenderPresent and SDL_Delay once per terrain element, i.e. once per innermost loop iteration. There is a pause of 1000 / FPS milliseconds between the rendering of every frame. Depending on the size of the terrain array (which you do not show in your posted code), the total time to render all of these frames could be many seconds. During this time, you are not processing events. As long as you don't process events, your window will be unresponsive. You only start processing events again in the outer loop, after the entire terrain array has been rendered.

    Instead of using SDL_Delay, I recommend that you use SDL_AddTimer. That way, a callback function will be called when it is time to render the next frame. Meanwhile, you can process events and your window will be responsive. I recommend that your callback function should do nothing more than call SDL_PushEvent with an application-defined event (registered using SDL_RegisterEvents), while your main thread is waiting for these and other possible events using SDL_WaitEvent.

    Alternatively, calling SDL_PumpEvents in between frames should be sufficient to keep the window responsive. But the solution mentioned in the above paragraph is cleaner if you want to be able to respond to events.

    Another alternative would be to use the function SDL_PollEvent which implicitly calls SDL_PumpEvents and allows you to check for new events without blocking.