I have a long-standing problem with the SDL_WaitEvent function because it doesn’t block waiting for a new event but continuously polls the event queue with a 1ms delay (see src/events/SDL_events.c). I understand that this is not a issue for a game but it is a problem for a desktop application that is not supposed to use the CPU when idle.
I have prepared a patch to fix this problem. It is currently located in the blocking-wait-event branch. You can look here the diff.
What the patch does is to actually make SDL_WaitEvent blocks waiting for an event. Please note that this doesn’t change the behavior of the library because in any case the function does not return until an event is available.
I would love to have some feedback about this change. Is it something that could be possibly accepted ?
I would appreciate also if someone could test my patch on Mac OS X. I tested on Windows and on Linux using the X11 video subsystem and everything seems fine.
Currently I lack also the implementation the the new video device function on many subsystems but I fallback to using SDL_WaitEventWithTimeout when not implemented.
It would be nice to see if you could plumb that timeout through as well, so it could be used for SDL_WaitEventTimeout() too. It looks like it should be possible for Cocoa and Win32 at least (though Win32 might need some clever use of SetTimer() to make timeouts work). Not sure about X11.
The other thing is that you’re not guaranteed that an OS window message/event will necessarily translate into an SDL event, so you need to make sure you’ve actually generated an SDL event before returning 1 (and it looks like you aren’t populating *event on success?) when your WaitNextEvent() returns.
You also need to handle the case where an event is put into the queue by some other thread in a call to SDL_PushEvent(). That should wake up your wait too.
It would be nice to see if you could plumb that timeout through as well, so it could be used for SDL_WaitEventTimeout() too. It looks like it should be possible for Cocoa and Win32 at least (though Win32 might need some clever use of SetTimer() to make timeouts work). Not sure about X11.
I have got the same idea when I was on the core for Mac OS X but now that you mentioned this idea I am pretty sure I am going to implement that.
For the xwindow system it seems there are no API to do it but I found I clever trick mentioned there.
The other thing is that you’re not guaranteed that an OS window message/event will necessarily translate into an SDL event, so you need to make sure you’ve actually generated an SDL event before returning 1 (and it looks like you aren’t populating *event on success?) when your WaitNextEvent() returns.
You are right, I need to fix my implementation to check if the event was actually translated into an SDL event and I need to populate *event. I need to add the
call to SDL_PeepEvent with the action GET_EVENT. Thank you!
You also need to handle the case where an event is put into the queue by some other thread in a call to SDL_PushEvent(). That should wake up your wait too.
The call to SDL_PeepEvent should be enough to check if we have actually an event I understand correctly.
Thank you very much for the feedback. I will come back to the list as soon as I have the new version.
The other thing is that you’re not guaranteed that an OS window message/event will necessarily translate into an SDL event, so you need to make sure you’ve actually generated an SDL event before returning 1 (and it looks like you aren’t populating *event on success?) when your WaitNextEvent() returns.
I have made the corrections with this commit in the same branch, blocking-wait-event. Seems to be okay on Linux with xwindow but I hope to get some feedback.
I’m still not sure how you plan to detect an event posted from another thread using SDL_PushEvent(). I’m assuming that won’t cause WaitNextEvent() to return, or will it?
You are right, I completely overlooked the “events from another thread” scenario. I have to work on this aspect to have a correct implementation, I have already some ideas.
By the way I also see another error in my recent commit, I need to call SDL_PeepEvent to get an available event if there is any before blocking with WaitNextEvent.
I will post again in the list when I have something new.
I think I have solved the problem to detect an event posted from another thread when inside the method WaitNextEvent. As before the work is in the blocking-wait-event branch. For your convenience here the complete diff of my modifications.
The way it work is pretty simple. When an event is added with SDL_PushEvent we call the new device method SendWakeupEvent. This latter just send a dummy event in the native event loop so that WaitNextEvent can wake up. As soon as it wakes up it will check the SDL’s event queue using SDL_PeepEvent and we find the new event pushed by SDL_PushEvent.
The implementation works on Windows and on XWindow. I tested with a newly introduced test, checkkeysthreas.c. On Windows it was trvial to implement while on X Window I had to introduce a secondary Display connection to safely send an event from another thread. I already tested this technique in the past and it works flawlessy.
I gave also an implementation for Mac OS X but it may not work or may be not even compile. Unfortunately I cannot test on Mac OS X.
If this work is fine and remains to do is a WaiNextEventWithTimeout to have a blocking call waiting for an event but with a timeout.
Of course, I overlooked this. Since your message I have been working to improve my patch. I discovered an unrelated problem that I have fixed now.
As for the joystick / sensor problem I have also a solution. What I do is that if a joystick or sensor is actually in use I do not block waiting for an event but I fall back to the old behavior of polling. I do not see any other solution except adding a separate thread to poll joysticks or sensors but it makes the code complex without any real advantage. In addition we would be left with the problem of polling the joystick without continuously running the CPU and I don’t know any way to do this.
In practice, for desktop application, not games, the joystick and sensors are not used so it seems fine to make this compromise. The downside is that games using the joystick will not see any benefit from this patch but I am ok with that.
I tested the new patch on Windows and on linux with xwindow in a variety of cases and it seems ok. The Cocoa implementation is still untested yet.
You may have a look at the new patch in the branch blocking-wait-event-submit, I have made a single commit that comprise all the changes I made since the beginning.
I have a plan for this, and it looks a little like this:
There are waitable handles that are platform-specific (HANDLE on win32, Unix file descriptors, etc). There’ll be a typedef for these somewhere.
Internally, SDL subsystems will report when there are new waitable handles (you connect to an X11 server? Report the handle. Open a joystick? Report the handle). SDL will manage the list of handles, and maintain it as handles come and go.
In SDL_WaitEvent(), SDL will wait for all these handles with the appropriate platform function (select/poll/WaitForMultipleObjects/whatever). It will take steps to handle timeout (including if one of the things we’re waiting on is an SDL timer). If the wait ends, SDL either polls for new events in the existing way, or sees if a timer event is ready to fire, and either goes back to sleep or returns.
SDL provides a new function, SDL_WaitEventPlusHandles (or a better name), which will let the app do the standard SDL wait but also supply its own handles … notably this is useful if you’re also waiting on a network socket, etc.
If a subsystem has an interface that can not produce a waitable handle, we have to drop back to polling, which happens a lot when there’s some sort of third-party library in the way that doesn’t expose the low-level handles to SDL. This works like now: sleep a little, poll, sleep a little, poll…ideally we avoid these interfaces or strive to upstream patches that expose these handles where possible.
In the end, you have something that is not only fully waitable inside SDL, but throughout your whole app, so it can use zero power when waiting for something external to happen (i.e. - a standard GUI app).
Been thinking about this for awhile, but haven’t written it yet. Was hoping to soon. I haven’t looked at @franko’s latest work, though, in case it’s going in that direction.
I like that idea. Can I just ask that whenever changes to the events system are contemplated, that the needs of multi-threaded applications are taken into account.
For example, I had assumed that the existing events mechanism was thread safe - it had always seemed to be on the main desktop platforms. But when I tried to port my app to Emscripten / Web Assembly (which has only recently gained support for multi-threading) I soon discovered that it isn’t!
I now have to wrap calls to SDL_PushEvent() and SDL_PeepEvents() etc. in a protective mutex, to fix this. That was easy enough, but protecting some of the events pushed internally by SDL (without modifying the source) wasn’t: I’ve had to resort to quite a nasty and fragile hack involving intercepting them with SDL_SetEventFilter().
So if the events system could be made genuinely thread-safe at the same time as other changes are made that would be very helpful.
I now have to wrap calls to SDL_PushEvent() and SDL_PeepEvents() etc. in a protective mutex, to fix this.
The event queue is thread safe (there’s an internal mutex)…it’s possible this mutex is disabled on Emscripten, under the (now sometimes incorrect) assumption that Emscripten is single-threaded.
I’m using the version of SDL2 that you get by specifying the USE_SDL=2 switch to the Emscripten compiler emcc, which supports multithreading, so that shouldn’t be the case.
Looking in the source, the mutex should only be disabled if SDL_THREADS_DISABLED is true, but if that was the case I wouldn’t expect to be able to create a second thread at all. So this may deserve some investigation.
I have improved the previous patch to be more accurate about the need of
sending the wake-up event. The new version is available in the blocking-wait-event-submit-3
branch.
I have submitted the patch also into the bugzilla system:
Now the wake-up event is sent only if the SDL_PushEvent call comes from a
different thread and only if the main thread is actually blocking. In addition
the wake-up event is sent only to a single window that is designated in advance
by the blocking thread.
In this way only one wake-up event is sent to a single one window and only when
needed. It is still possible that a spurious wake-up event is sent when the
blocking call to WaitNextEvent just terminated but the spurious event will makes
no harm as it will be ignored and does not translates into an SDL_Event.
The new version of the patch was required to work well with the xwindow system.
Without the corrections some errors happened on xwindow when used with
multi-threaded applications.
I would greatly appreciate if one of the SDL developers could review my
proposition.
I would also appreciate some help to review and test the OS X version and
contributions to implement the new methods for the Wayland video system. The two
methods that needs to be implemented are very simple and normally trivial to
implement.
Seems like an elegant solution I would love to see this implemented. It would be certainly better than the current approach: “poll continuously everything with a 1ms delay”.
I am just wondering if all source of events naturally provides an handle, I guess some will not and it should be created with a separate thread polling for events and reporting them to the handle. I am thinking for example to the hidapi interface.
I am also afraid that you proposition will need a long and uncertain time to be implemented if it is ever implemented at all. So while it is very attractive as a long term solution I think it may fail to provide a quick fix without redesigning the architecture of the events system.
No, my work doesn’t go in this direction, it is not that elegant. I stay with the current architecture of the event system and just provide a path to call a blocking call to wait for new events from the video system. I provide also a mechanism to wake-up the blocking thread if events are send from another threads.
This isn’t necessarily a problem, but it’s something I don’t like to see. SDL_THREADS_DISABLED is sometimes tested for being defined, for example in SDL_assert.c:
This could conceivably result in unwanted behavior it was defined but set to zero. I’ve not yet discovered where and when it is defined, can somebody point me to the relevant module?
It would be nice to see if you could plumb that timeout through as well, so it could be used for SDL_WaitEventTimeout() too. It looks like it should be possible for Cocoa and Win32 at least (though Win32 might need some clever use of SetTimer() to make timeouts work). Not sure about X11.
After more the two months, I have made the modifications that reduce the CPU usage to zero by making a blocking call to wait for an event including a timeout. So now both SDL_WaitEvent() and SDL_WaitEventTimeout() takes advantage of the reduced CPU usage.
I submitted a pull request on github:
and here is the related github issue:
Any feedback is welcome and if someone wants to test the Cocoa video driver implementation that would be very welcome. I wrote the code but is untested and I not even know if it compiles.
On the other side the code for Windows and X11 is tested and works. I have also used the previous version of the patch for the lite-xl editor since January without any issue at all.
