Proposed changes to the event system

So, what do you think of the plan I laid out?

I think I’d like to see pseudo code implementation for the various platforms
so I can see how your ideas translate into platform specific logic. Go ahead
and keep it high level, maybe comparing it to the existing implementation.

I’ll do better than pseudo code. I’ve got a Mac here, so I’ll convert
the X11 driver to my new SDL_VideoDevice interface, and I’ll convert
the Cocoa driver to use the “transition” method (which is functionally
the same as what we have now, but demonstrate that it should be
possible to convert drivers “en masse”). I will also add an
SDL_WaitEventTimeout to the API (only addition, is really optional,
but so many people have asked for that!).

This will give us an SDL_WaitEvent/SDL_WaitEventTimeout that is both
CPU-efficient and low latency, as well as reduce churn of the SDL
event queue (which might improve efficiency a very tiny bit), for the
X11 driver.

I won’t do the timers and additional event sources (like sockets)
right away, that should be enough to see where I’m going, the rest is
just “nice to have” that builds on top rather minimally.On Wed, Jan 14, 2009 at 1:15 AM, Sam Lantinga wrote:


http://pphaneuf.livejournal.com/

So, what do you think of the plan I laid out?

I think I’d like to see pseudo code implementation for the various platforms
so I can see how your ideas translate into platform specific logic. Go ahead
and keep it high level, maybe comparing it to the existing implementation.

I’ll do better than pseudo code.

I’d like pseudo code first, so I can make sure I understand what you’re
planning to do. I have some thoughts, but I want to make sure I fully
understand what you have in mind before I share them.

See ya!
-Sam Lantinga, Founder and President, Galaxy Gameworks LLC> On Wed, Jan 14, 2009 at 1:15 AM, Sam Lantinga wrote:

I’d like pseudo code first, so I can make sure I understand what you’re
planning to do. I have some thoughts, but I want to make sure I fully
understand what you have in mind before I share them.

Ok, first phase, let’s change the SDL_VideoDriver API. Replace this:

void (*PumpEvents) (_THIS);

With this:

int (*GetEvent) (_THIS, SDL_Event *event, int timeout);
void (*WakeGetEvent) (_THIS);

This will instantly break every drivers, unfortunately. Let’s fix
that. Let’s add a generic implementation of the GetEvent method:

/* SDL_PumpEvent almost verbatim */
static void SDL_OldPumpEvent(SDL_VideoDevice *_this,
void(oldpumpevent)(SDL_VideoDevice))
{
if (!SDL_EventThread) {
SDL_VideoDevice *_this = SDL_GetVideoDevice();

    if (_this)
        oldpumpevent(_this);

#if !SDL_JOYSTICK_DISABLED
/* Check for joystick state change */
if (SDL_numjoysticks && (SDL_eventstate & SDL_JOYEVENTMASK)) {
SDL_JoystickUpdate();
}
#endif
}
}

/* Takes lots of bits from SDL_PumpEvents and SDL_WaitEvent */
int SDL_GenericGetEvent(SDL_VideoDevice *_this, SDL_Event *event, int
timeout, void(oldpumpevent)(SDL_VideoDevice))
{
Uint32 expiration = UINT_MAX;

if (timeout > 0)
    expiration = SDL_GetTicks() + timeout;

while (1) {
    SDL_OldPumpEvent(_this, oldpumpevent);
    switch (SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_ALLEVENTS)) {
    case -1:
        return 0;
    case 1:
        return 1;
    case 0:
        if (timeout == 0 || SDL_GetTicks() >= expiration)
            return 0;
        else
            SDL_Delay(10);
    }
}

}

For Cocoa, that allows up to make the following implementations:

void Cocoa_GetEvent(_THIS, , SDL_Event *event, int timeout)
{
return SDL_GenericGetEvent(_this, event, timeout, Cocoa_PumpEvents);
}

We can leave the WakeGetEvent method NULL, it’s not really needed here
(but you could patch in a mechanism similar to Bob Pendleton’s
fastevents right there, that’d be a nice improvement!).

We make SDL_PumpEvent a no-op, it’s not really needed (I lie: it’s
probably what makes the SDL_GetKeyState work, but you’ll have to get
used to it and call SDL_PollEvent at least, you can’t just call
SDL_PumpEvents all the time and let the event queue fill up, can
you?).

We add the SDL_WaitEventTimeout:

int SDL_WaitEventTimeout(SDL_Event *event, int timeout)
{
if (_this) {
return _this->GetEvents(_this, event, timeout);
else
return -1; // we don’t have a device? bad!
}

We update SDL_PollEvent and SDL_WaitEvent:

int SDL_PollEvent(SDL_Event * event)
{
int retval = SDL_WaitEventTimeout(event, 0);

if (retval == -1)
    retval = 0;

return retval;

}

int SDL_WaitEvent(SDL_Event * event)
{
return SDL_WaitEventTimeout(event, -1);
}

SDL_PeekEvents should be modified to call “if (_this &&
_this->WakeGetEvents) _this->WakeGetEvents(_this);” for the case where
an event is added to the queue (with this generic implementation, it
doesn’t matter, but we’ll need it for the more clever ones).

Now, the SDL_VideoDevice API is in place.

I’ll go over the X11 driver a bit more quickly. :slight_smile:

The SDL_VideoData of the X11 driver gains “int event_sem_read; int
event_sem_write;”. When initializing the structure, those are filled
using the pipe(2) system call, then both are set to non-blocking mode
using fcntl(2).

The X11_WakeGetEvent is simply this one-liner: write(event_sem_write, “\0”, 1);

The X11_GetEvent is more tricky:

int X11_GetEvent(_this, SDL_Event *event, int timeout)
{
We call XFlush. This is needed because we might not do a XPending
or a XNextEvent for a while, so Xlib needs a bit of help, in case the
event we’re waiting for should be triggered by a command in the output
buffer.

/* If you're careful with the event_sem_read, you could actually

skip that part, but I’ll leave it like this for now. */
Try to get an event from the queue with SDL_PeepEvents(event, 1,
SDL_GETEVENT, SDL_ALLEVENTS).
If we do get one, we’re done, return 1.

Check if Xlib already has events for us, using

XEventsQueued(QueuedAlready). This never makes any system calls,
it’s basically just inspects the client-side queue and returns the
number of events already there.

If there is no event in the Xlib event queue, we do a select(),

using ConnectionNumber(display) to get the file descriptor of the X11
connection, putting that in the “readable” FD_SET, as well as the
event_sem_read. We also do the appropriate calculations for the
timeout. The select() might need to be retried, in case of EINTR (you
can get an arbitrary number of those, they’re harmless).

/* The following bit could be done conditionally upon
Read everything from the event_sem_read, until it returns

EWOULDBLOCK (you can use a largeish buffer, say 1K, we don’t care, we
just throw it away).
Try to get an event from the queue with SDL_PeepEvents(event, 1,
SDL_GETEVENT, SDL_ALLEVENTS).
If we do get one, we’re done, return 1.

If XPending() returns > 0, get an X11 event with XNextEvent() and

process it, using it to fill the SDL_Event *event parameter. If this
X11 event generates more than one SDL_Event, things get a bit more
complicated: we have to fill the SDL_Event *event with the first
event, and put the others in the queue, like we used to do. This step
is a bit hand-wavy, in reality it will require a good chunk of coding
(we can’t use things like SDL_SendMouseMotion, for example, at least,
not as it is today). I’ll put a NOTE at the bottom, related to this…

If XPending() does not return > 0, it just means that we timed out.

At this point, we should either have had an event, timed out or

had some kind of error.
}

Note that in the common case where one X11 event returns one SDL
event, we do not put the event in the queue. The select()'s timeout
acts as the SDL_Delay, but exits quicker if there is something to
return earlier (as much as the OS is able to, anyway, which is still
pretty good on Linux).

On Win32, the implementation of this would be fairly different, using
GetMessage, posting a message to itself to wake up instead of using a
pipe, etc… That will be the most work, making all those drivers
actually good, instead of using the cheap generic implementation…

This was of course pretty skimpy on the error handling, and there are
some bits where the ordering of the calls is rather important, so it
would need to be well commented (for example, the XFlush at the
beginning of X11_GetEvent, or the ordering of reading everything from
event_sem_read and looking at the SDL event queue). This becomes
especially important with timers, to avoid starvation (if you have an
idle timer event that is always expired, you’d never look at the X11
events if you processed them first, they have to be last).

NOTE about the “process the X11 event” step: This is a good example of
why a callback API would be nice. You could just call the callbacks,
one after the other, in the case where you get more than one event,
nothing special, and no need to put events in the SDL queue. At the
end of the day, it’s all equivalent, but all of those little
simplifications add up…

Of course, I expect a million little bugs to come out of the wazoo
with this, with things like SDL_GetKeyState, for example, but it’ll be
worth it, I think. :-)On Wed, Jan 14, 2009 at 3:39 AM, Sam Lantinga wrote:


http://pphaneuf.livejournal.com/

For Cocoa, that allows up to make the following implementations:

void Cocoa_GetEvent(_THIS, , SDL_Event *event, int timeout)
{
return SDL_GenericGetEvent(_this, event, timeout, Cocoa_PumpEvents);
}

Wouldn’t you want to take advantage of the callback nature of the SDL 1.3
cocoa implementation?

At this point, we should either have had an event, timed out or

had some kind of error.

XPending() will also return if here is some wire protocol pending.

Of course, I expect a million little bugs to come out of the wazoo
with this, with things like SDL_GetKeyState, for example, but it’ll be
worth it, I think. :slight_smile:

One thing that comes to mind is that generating the events should come
from the appropriate function in src/event, so that SDL’s internal state
is updated correctly.

Anyway, I’m interested in seeing how this plays out.

See ya,
-Sam Lantinga, Founder and President, Galaxy Gameworks LLC

void Cocoa_GetEvent(_THIS, , SDL_Event *event, int timeout)
{
return SDL_GenericGetEvent(_this, event, timeout, Cocoa_PumpEvents);
}

Wouldn’t you want to take advantage of the callback nature of the SDL 1.3
cocoa implementation?

Since I kind of gave up on the callback API and sticking to the
current API, no, can’t do much in the way of that. This was an example
of the “compatibility glue”, to quickly update video drivers without
too much work, so I didn’t do much at all, but the "proper"
implementation would still use [NSApp nextEventMatchingMask], with a
more appropriate untilDate parameter, and post itself a Cocoa user
events instead of using a pipe, things like that.

At this point, we should either have had an event, timed out or

had some kind of error.

XPending() will also return if here is some wire protocol pending.

I think there’s a word or two missing from that sentence. :slight_smile:

Yes, XPending() always return immediately (it uses non-blocking I/O),
but if there is no event in the input buffer, it will attempt to read
more events from the X11 connection. So we don’t really need to check
what if the X11 connection fd is in the FD_SET, we just try it out,
and if XPending() tells us there is no event, then that’s it, no
event, must have been a timeout.

Of course, I expect a million little bugs to come out of the wazoo
with this, with things like SDL_GetKeyState, for example, but it’ll be
worth it, I think. :slight_smile:

One thing that comes to mind is that generating the events should come
from the appropriate function in src/event, so that SDL’s internal state
is updated correctly.

Yeah, what I’m thinking is that the functions like SDL_SendKeyboardKey
that put an event in the queue, they should have an SDL_Event*
parameter added, and if it’s non-NULL, populate that event instead of
pushing one on the queue. All the existing call sites would just use a
NULL for the SDL_Event*.

But the typical very, very newbie program that just uses
SDL_GetKeyState and SDL_PumpEvents, that program is borked. :frowning:

Anyway, I’m interested in seeing how this plays out.

So am I. :-)On Wed, Jan 14, 2009 at 10:11 AM, Sam Lantinga wrote:


http://pphaneuf.livejournal.com/