A way to make keyboard event queue both responsive and not take whole CPU power

Question

I am making an Sdl game, it's 2d shooter. I am using SDL to import surfaces and OpenGL to draw them on the screen (doing so because it works way faster than just SDL). I've got two threads running, one for processing stuff and rendering, and another one for input. Basically, the processing one is taking 1-2% of my CPU, while the input loop takes 25% (on quad-core, so it's 1 full core). I tried doing SDL_Delay(1) before each while (SDL_PollEvent(&keyevent)) and it works! Reduces CPU load to 3% for whole process. However, there's a nasty side effect. The whole program input is handicapped: it doesn't detect all of the key pressed, and for example, to make the character move, it sometimes takes up to 3 seconds of bashing keyboard for it to react.

I've also tried solving it by using SDL_PeepEvent() and SDL_WaitEvent(), however, it's causing the same (very long!) delay.

Event loop code:

void GameWorld::Movement()
{
    SDL_Event keyevent;
    bool x1, x2, y1, y2, z1, z2, z3, m;         // Booleans to determine the
    x1 = x2 = y1 = y2 = z1 = z2 = z3 = m = 0;   // movement direction
    SDL_EnableKeyRepeat(0, 0);
    while (1)
    {
        while (SDL_PollEvent(&keyevent))
        {
            switch(keyevent.type)
            {
            case SDL_KEYDOWN:
                switch(keyevent.key.keysym.sym)
                {
                case SDLK_LEFT:
                    x1 = 1;
                    x2 = 0;
                    break;
                case SDLK_RIGHT:
                    x1 = 0;
                    x2 = 1;
                    break;
                case SDLK_UP:
                    y1 = 1;
                    y2 = 0;
                    break;
                case SDLK_DOWN:
                    y1 = 0;
                    y2 = 1;
                    break;
                default:
                    break;
                }
                break;
            case SDL_KEYUP:
                switch(keyevent.key.keysym.sym)
                {
                case SDLK_LEFT:
                    x1 = x2 = 0;
                    break;
                case SDLK_RIGHT:
                    x1 = x2 = 0;
                    break;
                case SDLK_UP:
                    y1 = y2 = 0;
                    break;
                case SDLK_DOWN:
                    y1 = y2 = 0;
                    break;
                default:
                    break;
                }
                break;
            case SDL_QUIT:
                PrintToFile("The game was closed manually.\n");
                CleanUp();
                return;
                break;
            default:
                break;
            }
        }
        m = x1 || x2 || y1 || y2;
        if (m)   // if any button is pushed down, calculate the movement
        {        // direction and assign it to the player
            z1 = (x1 || x2) && (y1 || y2);
            z2 = !x1 && (x2 || y2);
            z3 = (!y1 && x1) || (!y2 && x2);
            MainSurvivor->SetMovementDirection(4 * z1 + 2 * z2 + z3);
        }
        else    // if no button is pushed down, reset the direction
            MainSurvivor->SetMovementDirection(-1);
    }
}

Code for calculation/render loop:

void GameWorld::GenerateCycles()
{
    int Iterator = 0;
    time_t start;   
    SDL_Event event;

    Render();
    _beginthread(MovementThread, 0, this);
    while (1)
    {
            // I know I check this in input loop, but if I comment
        SDL_PollEvent(&event);   // out it from here, that loop cannot
        if (event.type == SDL_QUIT)  // see any of the events (???)!
        {
            PrintToFile("The game was closed manually.\n");
            CleanUp();
        }                            // It never closes through here though

        start = clock();
        Iterator++;
        if (Iterator >= 232792560)
            Iterator %= 232792560;
        MainSurvivor->MyTurn(Iterator);
        for (unsigned int i = 0; i < Survivors.size(); i++)
        {
            Survivors[i]->MyTurn(Iterator);
            if (Survivors[i]->GetDiedAt() != 0 && Survivors[i]->GetDiedAt() + 25 < clock())
            {
                delete Survivors[i];
                Survivors.erase(Survivors.begin() + 5);
            }
        }
        if (Survivors.size() == 0)
            SpawnSurvivors();

        for (int i = 0; i < int(Zombies.size()); i++)
        {
            Zombies[i]->MyTurn(Iterator);
            if (Zombies[i]->GetType() == 3 && Zombies[i]->GetDiedAt() + 25 < Iterator)
            {
                delete Zombies[i];
                Zombies.erase(Zombies.begin() + i);
                i--;
            }
        }
        if (Zombies.size() < 3)
            SpawnZombies();

            // No need to render every cycle, gameplay is slow
        if (Iterator % 2 == 0)
            Render();

        if (Interval - clock() + start > 0)
            SDL_Delay(Interval - clock() + int(start));
    }
}

Does anyone have any ideas?

Answer 1

I'm not really experienced with SDL or game programming, but here are some random ideas:

Reacting on state changes

Your code:

while (1)
{
    while (SDL_PollEvent(&keyevent))
    {
        switch(keyevent.type)
        {
            // code to set keyboard state
        }
    }

    // code to calculate movement according to keyboard state
    // then act on that movement
}

This means that no matter the fact nothing is happening on your keyboard, you are calculating and setting data.

If setting the data is expensive (hint: synchronized data), then it will cost you even more.

SDL_WaitEvent : measuring state

You must wait for an event to happen, instead of the spinning you wrote which causes 100% usage of one processor.

Here's a variation of the event loop I wrote for a test at home:

while(true)
{
    // message processing loop
    ::SDL_Event event ;

    ::SDL_WaitEvent(&event) ; // THIS IS WHAT IS MISSING IN YOUR CODE

    do
    {
        switch (event.type)
        {
            // etc.
        }
    }
    while(::SDL_PollEvent(&event)) ;

    // re-draw the internal buffer
    if(this->m_isRedrawingRequired || this->m_isRedrawingForcedRequired)
    {
        // redrawing code
    }

    this->m_isRedrawingRequired = false ;
    this->m_isRedrawingForcedRequired = false ;
}

Note : This was single threaded. I'll speak about threads later.

Note 2 : The point about the the two "m_isRedrawing..." boolean is to force redrawing when one of those booleans are true, and when the timer asks the question. Usually, there is no redrawing.

The difference between my code and yours is that at no moment you let the thread "wait".

Keyboard events

There is a problem, I guess, with your handling of keyboard events.

Your code:

        case SDL_KEYDOWN:
            switch(keyevent.key.keysym.sym)
            {
            case SDLK_LEFT:
                x1 = 1;
                x2 = 0;
                break;
            case SDLK_RIGHT:
                x1 = 0;
                x2 = 1;
                break;
            // etc.
            }
        case SDL_KEYUP:
            switch(keyevent.key.keysym.sym)
            {
            case SDLK_LEFT:
                x1 = x2 = 0;
                break;
            case SDLK_RIGHT:
                x1 = x2 = 0;
                break;
            // etc.
            }

Let's say You press LEFT, and then RIGHT, then unpress LEFT. What I'd expect is:

1. press LEFT : the character goes left
2. press RIGHT : the character stops (as both LEFT and RIGHT are pressed)
3. unpress LEFT : the character goes right, because RIGHT is still pressed

In your case, you have:

1. press LEFT : the character goes left
2. press RIGHT : the character goes right (as now LEFT is ignored, with x1 = 0)
3. unpress LEFT : the character stops (because you unset both x1 and x2.), despite the fact RIGHT is still pressed

You're doing it wrong, because:

1. You're reacting immdiately to an event, instead of using a timer to react to a situation every nth millisecond
2. you are mixing events together.

I'll find the link later, but what you should do is have an array of boolean states for pressed keys. Something like:

// C++ specialized vector<bool> is silly, but...
std::vector<bool> m_aKeyIsPressed ;

You initialize it with the size of available keys:

m_aKeyIsPressed(SDLK_LAST, false)

Then, on key up event:

void MyContext::onKeyUp(const SDL_KeyboardEvent & p_oEvent)
{
    this->m_aKeyIsPressed[p_oEvent.keysym.sym] = false ;
}

and on key down:

void MyContext::onKeyDown(const SDL_KeyboardEvent & p_oEvent)
{
    this->m_aKeyIsPressed[p_oEvent.keysym.sym] = true ;
}

This way, when you check at regular intervals (and the when you check part is important), you know the exact instantaneous state of the keyboard, and you can react to it.

Threads

Threads are cool but then, you must know exactly what you are dealing with.

For example, the event loop thread calls the following method:

MainSurvivor->SetMovementDirection

The resolution (rendering) thread calls the following method:

MainSurvivor->MyTurn(Iterator);

Seriously, are you sharing data between two different threads?

If you are (and I know you are), then you have either:

1. if you didn't synchronize the accesses, a data coherence problem because of processor caches. Put it simply, there's no guarantee one data set by one thread will be seen as "changed" by the other in a reasonable time.
2. if you did synchronize the accesses (with a mutex, atomic variable, etc.), then you have a performance hit because you are (for example) locking/unlocking a mutex at least once per thread per loop iteration.

What I would do instead is communicate the change from one thread to another (via a message to a synchronized queue, for example).

Anyway, threading is an heavy issue, so you should familiarize with the concept before mixing it with SDL and OpenGL. Herb Sutter's blog is a marvelous collection of articles on threads.

What you should do is:

1. Try to write the thing in one thread, using events, posted messages, and timers.
2. If you find performance issues, then move the event or the drawing thread elsewhere, but continue to work with events, posted messages, and timers to communicate

P.S.: What's wrong with your booleans?

You're obviously using C++ (e.g. void GameWorld::Movement()), so using 1 or 0 instead of true or false will not make your code clearer or faster.

Answer 2

If you initialized SDL on GameWorld::GenerateCycles()'s thread and MovementThread is calling GameWorld::Movement() then you have a problem:

• Don't call SDL video/event functions from separate threads

Answer 3

Have you tried using something like usleep(50000) instead of delay(1)?

This would make your thread sleep for 50 msecs between polling the queue, or equivalently, you would check the queue 20 times per second.

Also, what platform is this on: Linux, Windows?

On Windows you may not have usleep(), but you can try select() as follows:

struct timeval tv;
tv.tv_sec = 0;
tv.tv_usec = 50000;
select(0, NULL, NULL, NULL, &tv);

Another suggestion is to try polling in a tight loop until it stops returning events. Once no events are pending, proceed with sleeping for 50 msecs between polls until it starts returning events again.

Answer 4

I'd suggest looking into SDL_EventFilter and the related functions. It's not a polling queue input method, so it doesn't require stalling, although, if I remember correctly, it doesn't happen on the main thread, which may be exactly what you need for performance, but might complicate the code.