Improved SDL_PollEvent usage

[0x1F9F1]

This PR changes the behaviour of SDL_WaitEventTimeout (used by SDL_PollEvent) to reduce overhead, and avoid infinite loops.

Description

Currently when polling for events you can end up in an infinite loop. There are two main reasons for this:

• SDL_WaitEventTimeout unconditionally calls SDL_PumpEvents, even if there are already events available.
• SDL_WaitEventTimeout has no understanding of a poll cycle.

To fix this, there are two main changes:

• Avoid calling SDL_PumpEvents unless the event queue is actually empty.
• Use a sentinel event to mark the end of a poll cycle.

Alternatives

Unfortunately both of these changes could alter the behaviour of existing applications, so the following changes may be needed:

• Add a hint to preserve some/all of the previous behaviour.
• Only avoid pumping if timeout == 0.
• If you reach the sentinel, but there are more non-pump events (added in the middle of a poll loop), add the sentinel back to the end of the queue.

Existing Issue(s)

#4792 and #4376 describe the problems with the current event-poll cycle, though I felt part of #4792's approach is sub-optimal.

[0x0ade]

I've just tested your commits on testsprite2 with my 1kHz mouse and Spy++ attached to it, and the main problem which I've meant to solve with #4792 is also fixed with this PR 🎉

Should I keep my PR open as to discuss the remaining mouse move squishing changes?

[0x1F9F1]

I've just tested your commits on testsprite2 with my 1kHz mouse and Spy++ attached to it, and the main problem which I've meant to solve with #4792 is also fixed with this PR 🎉

Should I keep my PR open as to discuss the remaining mouse move squishing changes?

Not too sure. I assume you can still end up with multiple (but not many) WM_MOUSEMOVE events when calling WIN_PumpEvent, but MAX_NEW_MESSAGES should already handle getting stuck in there. IIRC the main source of event spam is WM_INPUT (as mentioned in #4376), as those aren't batched like WM_MOUSEMOVE. Merging all those would certainly help, though that's quite a bit more complicated.

[0x1F9F1]

After searching through GitHub for uses of SDL_PollEvent, there is an alarming amount of strange/incorrect usage (mostly in beginner projects), such as:

• Only calling it once per frame
• Not checking the return value
• Calling SDL_PollEvent(NULL) in a loop
• Polling until a certain event (i.e a key press), and pushing the rest back into the queue

I'd say those first 3 issues are broken by design, but the last one (and similar uses of user-added events inside a poll cycle) could be an issue, so I decided it's better to err on the side of caution and ignore the sentinel if there are still more events in the queue.

[DomGries]

This demonstrates why SDL_PollEvent is flawed by design and I can't think of a better way to fix it than shown in this PR. Good job!

Btw for anyone reading I recommend to use the better designed and more performant SDL_PumpEvents + SDL_PeepEvents like so (modified version of @meyraud705's code to really only call SDL_PeepEvents once if all events fit in the buffer):

#define SIZE_EVENTS 64
SDL_Event events[SIZE_EVENTS];
while (1) { // main game loop
    SDL_PumpEvents();
    int eventCount;
    do {
        eventCount = SDL_PeepEvents(&events[0], SIZE_EVENTS, SDL_GETEVENT, SDL_FIRSTEVENT, SDL_LASTEVENT);
        for (int i = 0; i < eventCount; ++i) {
            switch (events[i].type) {
                 /* handle event */
            }
        }
    } while (eventCount == SIZE_EVENTS);
}

[slouken]

You should always call SDL_PumpEvents() at least once through the function, otherwise you might starve the OS events if there's another thread pushing events onto the queue.

[0x1F9F1]

You should always call SDL_PumpEvents() at least once through the function, otherwise you might starve the OS events if there's another thread pushing events onto the queue.

SDL_PumpEvents will still be called at least once per cycle (where a cycle is calling SDL_PollEvent in a loop it returns false).

More specifically, it will be called when there are no events left in the SDL queue. The difference is that if SDL_PumpEvents then produces any new events, an extra sentinel event is also added.

When that sentinel event is reached there are two possibilities:
If more SDL events have been added after the sentinel, then the sentinel is ignored and SDL_PollEvent will continue processing those events (repeating the cycle).
Otherwise, since we know SDL_PumpEvents has already been called within this cycle (otherwise the sentinel wouldn't exist), return false (and therefore ending the cycle).