SFML community forums

General => Feature requests => Topic started by: l0calh05t on February 02, 2008, 12:50:33 am

Title: Separate Window.Display from event polling
Post by: l0calh05t on February 02, 2008, 12:50:33 am
Would it be possible to split Window.Display from event polling (and expose polling in the public interface)? This could be done via an auto-poll flag (which should be true by default, as not to break the api).

Furthermore it would be *really* nice if I could unbind the current rendercontext, to rebind it again in a different thread.

The first point would separate rendering&input more, and it would become possible to pause rendering (including unnecessary buffer clearing & swapping) when the window is minimized, without automatically also stopping input from being polled (basically resulting in a crash/unresponsive window)

The second point (combined with the first) would make it possible to separate Rendering into a different Thread which make far more sense than having those in the same thread in a multi threading environment (which is something we honestly can't ignore anymore), because rendering has nothing to do with input handling and this way the events could be polled directly in the thread that actually uses the events (which otherwise would have to be sent to the other thread first, possibly requiring extra synchronization).

I really hope you make this possible, as I really like this library, but also need these features.

For a small win32 example of what I want to do, see below:

Code: [Select]

#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include <GL/gl.h>

LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam);
void EnableOpenGL(HDC hDC, HGLRC hRC);
void DisableOpenGL(HDC hDC, HGLRC hRC);

volatile bool quit = false;
volatile bool paused = false;

struct GlThreadData
{
HDC hDC;
HGLRC hRC;
};

DWORD WINAPI GlThread(LPVOID arg1)
{
GlThreadData d = *reinterpret_cast<GlThreadData*>(arg1);
HDC hDC = d.hDC;
HGLRC hRC = d.hRC;
float theta = 0.0f;

EnableOpenGL( d.hDC, d.hRC );

glClearColor( 0.0f, 0.0f, 0.0f, 0.0f );

while(!quit)
{
glClear( GL_COLOR_BUFFER_BIT );

glPushMatrix();
glScalef(0.9f,0.9f,0.9f);
glRotatef( theta, 0.0f, 0.0f, 1.0f );
glBegin( GL_TRIANGLES );
glColor3f( 1.0f, 0.0f, 0.0f ); glVertex2f( 0.0f, 1.0f );
glColor3f( 0.0f, 1.0f, 0.0f ); glVertex2f( 0.87f, -0.5f );
glColor3f( 0.0f, 0.0f, 1.0f ); glVertex2f( -0.87f, -0.5f );
glEnd();
glPopMatrix();

SwapBuffers( d.hDC );

if(!paused)
theta += 1.0f;
}

DisableOpenGL( d.hDC, d.hRC );

return 0;
}

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance,
  LPSTR lpCmdLine, int iCmdShow)
{
WNDCLASS wc;
HWND hWnd;
MSG msg;

wc.style = CS_OWNDC;
wc.lpfnWndProc = WndProc;
wc.cbClsExtra = 0;
wc.cbWndExtra = 0;
wc.hInstance = hInstance;
wc.hIcon = LoadIcon( NULL, IDI_APPLICATION );
wc.hCursor = LoadCursor( NULL, IDC_ARROW );
wc.hbrBackground = (HBRUSH)GetStockObject( BLACK_BRUSH );
wc.lpszMenuName = NULL;
wc.lpszClassName = L"MsgPumpThread";
RegisterClass(&wc);

hWnd = CreateWindow(
L"MsgPumpThread", L"Threaded OpenGL / Windows Message Pump",
WS_CAPTION | WS_POPUPWINDOW | WS_VISIBLE,
0, 0, 300, 300,
NULL, NULL, hInstance, NULL );

PIXELFORMATDESCRIPTOR pfd;
int format;

HDC hDC = GetDC( hWnd );

ZeroMemory( &pfd, sizeof( pfd ) );
pfd.nSize = sizeof( pfd );
pfd.nVersion = 1;
pfd.dwFlags = PFD_DRAW_TO_WINDOW | PFD_SUPPORT_OPENGL | PFD_DOUBLEBUFFER;
pfd.iPixelType = PFD_TYPE_RGBA;
pfd.cColorBits = 24;
pfd.cAlphaBits = 8;
pfd.cDepthBits = 24;
pfd.cStencilBits = 8;
pfd.iLayerType = PFD_MAIN_PLANE;
format = ChoosePixelFormat( hDC, &pfd );
SetPixelFormat( hDC, format, &pfd );

HGLRC hRC = wglCreateContext( hDC );

GlThreadData gtd = { hDC, hRC };

HANDLE hGlThread;
hGlThread=CreateThread(0,0,&GlThread,&gtd,0,0);

while(!quit)
{
if(!GetMessage(&msg, NULL, 0, 0))
{
quit = true;
}
else
{
TranslateMessage(&msg);
DispatchMessage(&msg);
}
}

WaitForSingleObject(hGlThread,INFINITE);

wglDeleteContext( hRC );
ReleaseDC( hWnd, hDC );

DestroyWindow( hWnd );

return msg.wParam;

}

LRESULT CALLBACK WndProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
{

switch (message)
{

case WM_CREATE:
return 0;

case WM_CLOSE:
PostQuitMessage( 0 );
return 0;

case WM_DESTROY:
return 0;

case WM_KEYDOWN:
switch ( wParam )
{
case VK_SPACE:
paused = !paused;
return 0;

case VK_ESCAPE:
PostQuitMessage(0);
return 0;

}
return 0;

default:
return DefWindowProc( hWnd, message, wParam, lParam );
}

}

void EnableOpenGL(HDC hDC, HGLRC hRC)
{
wglMakeCurrent( hDC, hRC );
}

void DisableOpenGL(HDC hDC, HGLRC hRC)
{
wglMakeCurrent( NULL, NULL );
}
Title: Separate Window.Display from event polling
Post by: T.T.H. on February 02, 2008, 09:27:11 am
Valid point, nevertheless some thoughts from me:

In case you have a GUI, mouse event handling would be directly depending on the graphics and making two threads (events, graphics) to access the GUI data in a thread safe way (mutexes) might actually slow down your two threads again.

I'm not sure about but it might be that OpenGL must handled in the main thread of the process because otherwise it might produce very "wicked" results on some graphic card drivers.
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 02, 2008, 10:09:08 am
Quote from: "T.T.H."
Valid point, nevertheless some thoughts from me:

In case you have a GUI, mouse event handling would be directly depending on the graphics and making two threads (events, graphics) to access the GUI data in a thread safe way (mutexes) might actually slow down your two threads again.

I'm not sure about but it might be that OpenGL must handled in the main thread of the process because otherwise it might produce very "wicked" results on some graphic card drivers.


Mouse event handling does *not* depend on graphics, even mouse cursor display only depends on graphics if you use a sprite as cursor, the standard windows cursor (and i believe also X11 and OSX) is independent from rendering. You can try this with the small example i posted by having the window cleared 10000 times every frame, which will slow down rendering to ~1fps, but the cursor will still move correctly.

And OpenGL does not have to be handled in the main thread (just try the example), what you do have to do is make the context current in the rendering thread (and only there, so if it was current in another thread before, you'll have to call wglMakeCurrent( NULL, NULL ) (for Win32) in the other thread first)

EDIT:
And why would event handling and graphics both have to access gui data? Only the gui manager needs to access gui data, the event handler only has to tell the manager what happened (if necessary) and this could be done via a lockless queue (basically forwarding preprocessed events to all listeners of a certain event type, so the renderer for example only gets resize events)
Title: Separate Window.Display from event polling
Post by: Laurent on February 03, 2008, 10:01:49 am
That's interesting.

I guess this kind of feature won't be used by a lot of people, but you're right, that would make things much more flexible.

I'll try to integrate this to the sf::Window interface, I'll keep you informed ;)
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 03, 2008, 10:39:51 am
Quote from: "Laurent"
That's interesting.

I guess this kind of feature won't be used by a lot of people, but you're right, that would make things much more flexible.


Yeah, that's why I would suggest to provide a flag if the events are automatically polled (and that it should be set by default). That way, those who don't use the added flexibility don't have to care.

Quote
I'll try to integrate this to the sf::Window interface, I'll keep you informed ;)


Thank you very much, I hope this feature will be in SFML in the near future, because I'd really like to continue using it.
Title: Separate Window.Display from event polling
Post by: Laurent on February 03, 2008, 02:27:59 pm
I've just finished the modifications.

The event handling is now automatically independant from Display() (you don't have to do anything).

The window's context can be unbound with SetActive(false) (SetActive(true) replaces SetCurrent).

You can get the modifications with SVN.
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 12:41:32 am
Quote from: "Laurent"
I've just finished the modifications.

The event handling is now automatically independant from Display() (you don't have to do anything).

The window's context can be unbound with SetActive(false) (SetActive(true) replaces SetCurrent).

You can get the modifications with SVN.


Wow, that was fast! I checked out the newest revision, and it seems to work as intended, but I am unsure if one spot in the code is entirely safe on all platforms (and technically it's not thread safe either), I am talking about these lines in sf::Window::Display():

Code: [Select]

    // Make sure events have been processed
    if (myWindow && !myEventsProcessed)
    {
        myWindow->DoEvents();
    }
    myEventsProcessed = false;


On windows myWindow->DoEvents(); will do nothing if called from the wrong thread, dunno how it is with other platforms, you should probably check if it is the same.
Furthermore, setting myEventsProcessed to false at this spot, is technically not threadsafe, as you might be reading/writing from two threads. (Rendering thread & input thread)

But in any case, it does work (at least on Win32), so: Merci beaucoup :-)
Title: Separate Window.Display from event polling
Post by: Laurent on February 04, 2008, 02:14:25 am
Quote
On windows myWindow->DoEvents(); will do nothing if called from the wrong thread, dunno how it is with other platforms, you should probably check if it is the same.

Yep, that's what I was planning to do. I don't know if it has the same behavior, but I can't see any reason it would do something bad.

Quote
Furthermore, setting myEventsProcessed to false at this spot, is technically not threadsafe, as you might be reading/writing from two threads. (Rendering thread & input thread)

You're right again, the code is not multithread-safe. That will probably be the next step to make sf::Window robust with this new feature.

Quote
Merci beaucoup

De rien ;)
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 09:49:22 am
Quote from: "Laurent"
Yep, that's what I was planning to do. I don't know if it has the same behavior, but I can't see any reason it would do something bad.


Assuming it does poll events on some platform, this would cause multiple threads to write to one std::queue, which would be very bad. So, it is worth checking imo.

Quote
You're right again, the code is not multithread-safe. That will probably be the next step to make sf::Window robust with this new feature.


Using the architecture I suggested, this would be solved in the following way:

1) User calls Window.setAutoPollEvents(false);
2) Now, in the main thread, events can by polled via Window.pollEvents() (maybe also Window.waitEvents() could be provided)
3) Window.Display doesn't touch events as long as auto poll is still false, so no thread safety issues there.

This is a little more work for people who want to use it the way I want to, but for those who do not use this feature, nothing changes. And it is thread safe. Only disadvantage: If autopoll is (accidentally) set to false, and the user forgets to poll events manually, events will not be handled.

PS: One more tiny little suggestion: Add the event types Iconified and Restored. (That way the game could for example be partially or fully suspended when the window is iconified)
Title: Separate Window.Display from event polling
Post by: Laurent on February 04, 2008, 10:22:56 am
If the only thing is to remove possible event handling from Display(), it can already be done without changing the code. As you say, the only drawback is that events won't be polled if the user forgets to call GetEvent. But I don't get the point of adding functions such as SetAutoPollEvents or PollEvents.

I also need to check all the event handling code, to make sure everything else is thread-safe.

Quote
PS: One more tiny little suggestion: Add the event types Iconified and Restored. (That way the game could for example be partially or fully suspended when the window is iconified)

If all systems support these events, I'll add it ;)
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 11:13:51 am
Quote from: "Laurent"
If the only thing is to remove possible event handling from Display(), it can already be done without changing the code. As you say, the only drawback is that events won't be polled if the user forgets to call GetEvent. But I don't get the point of adding functions such as SetAutoPollEvents or PollEvents.


Ok, assume you remove event polling from Display (this is what you are suggesting, correct?), then how does GetEvent know if it already polled recently? (Hmm... maybe if events are polled whenever the Queue is empty... that should work) Assuming this problem is solved, what if someone only uses Asynchronous input? (IsKeyDown etc.) Eh... well, actually those have to do GetEvent anyways as they probably also need to know when the window is closed, so I guess that is no real problem.

Solving the problem with SetAutoPollEvents and PollEvents is merely what I would have done, but if you manage to solve this without making the API more complex  that would obviously be better.

EDIT:
I rewrote GetEvent to

Code: [Select]

bool Window::GetEvent(Event& EventReceived)
{
// Let the window implementation process incoming events
if (myWindow && myEvents.empty())
{
myWindow->DoEvents();
}

    // Pop first event of queue, if not empty
    if (!myEvents.empty())
    {
        EventReceived = myEvents.front();
        myEvents.pop();

        return true;
    }

    return false;
}


and removed

Code: [Select]

    // Make sure events have been processed
    if (myWindow && !myEventsProcessed)
    {
        myWindow->DoEvents();
    }
    myEventsProcessed = false;


from Display, and it seems to work and should be threadsafe (unless GetEvent is called from different threads but that would be pretty nonsensical anyway)
Title: Separate Window.Display from event polling
Post by: Laurent on February 04, 2008, 01:38:50 pm
That looks good :)

But I must admit I'm still not sure about giving up event handling in Display. That's the only function which I'm sure will be called each frame ; if I remove event handling from it, I can't be 100% sure that events will be polled. Even if, as you said, most people will use GetEvent at least to catch the Closed event.
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 01:48:56 pm
Understandable, but even if you do keep Event handling in Display, not calling GetEvent will still cause major problems. Why? Well, you'll have a queue to which you are consistently appending events at a fairly high rate, and if they are never removed from the queue this will cause one big memory leak...
Title: Separate Window.Display from event polling
Post by: Laurent on February 04, 2008, 02:01:14 pm
You're perfectly right. That finished to convince me, I just changed the code ;)
Title: Separate Window.Display from event polling
Post by: Aszarsha on February 04, 2008, 02:37:04 pm
It's not actually a memory leak since it's the intended behavior.

Splitting event handling from window display is a great suggestion, I've thought of it a few times, but your model isn't really adequate.

Polling the system through system calls (even if highly optimized) every time GetEvent is called is all but efficient.
I understand backward compatibility issues, but I submitted another suggestion a few weeks ago which aimed at the same system of SFML, so :
There could be two functions, PollEvents() et WaitEvents(), which would poll the event loop of the system. The two would add events to SFML events queue, as it currently is. WaitEvents would act as PollEvents do, unless the first poll is empty, if so, WaitEvents would enter in passive wait from the system.
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 03:08:45 pm
Quote from: "Aszarsha"
It's not actually a memory leak since it's the intended behavior.


Certainly not intended by the person who only wants to use asynchronous input :wink:

Quote
Splitting event handling from window display is a great suggestion, I've thought of it a few times, but your model isn't really adequate.

Polling the system through system calls (even if highly optimized) every time GetEvent is called is all but efficient.
I understand backward compatibility issues, but I submitted another suggestion a few weeks ago which aimed at the same system of SFML, so :
There could be two functions, PollEvents() et WaitEvents(), which would poll the event loop of the system. The two would add events to SFML events queue, as it currently is. WaitEvents would act as PollEvents do, unless the first poll is empty, if so, WaitEvents would enter in passive wait from the system.


The way it is done right now, is that GetEvent only polls system events when the queue is empty, so on average you will be polling system events twice every event loop (at the beginning and at the end). It is highly unlikely that this will become a bottleneck for your app.

EDIT: Which of your threads are you talking about? I only found http://sfml.sourceforge.net/forum-fr/viewtopic.php?t=419 but I assume you mean a different one?
Title: Separate Window.Display from event polling
Post by: Aszarsha on February 04, 2008, 03:35:18 pm
Quote from: "l0calh05t"
The way it is done right now, is that GetEvent only polls system events when the queue is empty, so on average you will be polling system events twice every event loop (at the beginning and at the end). It is highly unlikely that this will become a bottleneck for your app.
Ok. I haven't read the code and supposed poll was done every call. :oops:
Sorry.

I'll never have my passive waiting ! :cry:

EDIT : Yep, it's a different one ;) Look at this one : Attente passive (http://sfml.sourceforge.net/forum-fr/viewtopic.php?t=429).
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 03:53:18 pm
Quote from: "Aszarsha"
Ok. I haven't read the code and supposed poll was done every call. :oops:
Sorry.

I'll never have my passive waiting ! :cry:

EDIT : Yep, it's a different one ;) Look at this one : Attente passive (http://sfml.sourceforge.net/forum-fr/viewtopic.php?t=429).


Ah, I see. Although I don't need passive waiting myself I see why you'd like to have it.

Under Win32 it's easy enough to wait for events, just use GetMessage instead of PeekMessage, similar commands should exist for X11 and OSX as well
Title: Separate Window.Display from event polling
Post by: Aszarsha on February 04, 2008, 04:06:48 pm
Quote from: "l0calh05t"
Ah, I see. Although I don't need passive waiting myself I see why you'd like to have it.
Youpi ! I'm not alone ! :)
Quote from: "l0calh05t"
Under Win32 it's easy enough to wait for events, just use GetMessage instead of PeekMessage, similar commands should exist for X11 and OSX as well
That's why i've asked for it under SFML. ;)

Laurent, pliiiz ! :P
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 06:19:07 pm
btw, if you mainly want to save cpu time when no events are received you could do the following:

instead of:

Code: [Select]

while(running)
{
  while(GetEvent())
  {
    // process event
  }
}


which is obviously inefficient if you don't do anything except for processing events do the following:

Code: [Select]

while(running)
{
  if(GetEvent())
  {
     // process event
  }
  else
  {
    Sleep(0.01f); // sleep for 10ms
  }
}


This should lower cpu usage from 100% to ~2% (as long as there is no excessive processing associated with any specific event)

But if you really, really need passive event polling there are 3 options:
1) Add it yourself (should be easy enough)
2) Use SDL, despite it's somewhat large size, C-Style interface and LGPL license
3) Use GLFW, despite it's lack of support for multiple windows, C-Style interface and being only a framework for creating a valid GL context. (No audio, high-level graphics objects etc.)
Title: Separate Window.Display from event polling
Post by: Aszarsha on February 04, 2008, 07:03:55 pm
Hehe, that's what I actually do. ;)

I don't really have time to add things to SFML, I don't have time for my own projects... :x
SDL ? No, thanks.
And as I use 2d drawing facilities of SFML, GLFW is not really usefull (as you said).

Thanks for your suggestions l0calh05t. :)
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 07:11:44 pm
Quote from: "Aszarsha"
Hehe, that's what I actually do. ;)

I don't really have time to add things to SFML, I don't have time for my own projects... :x
SDL ? No, thanks.
And as I use 2d drawing facilities of SFML, GLFW is not really usefull (as you said).


Thought as much. (Who in the right mind would use SDL if they can use SFML...)

Quote
Thanks for your suggestions l0calh05t. :)


No problem.
Title: Separate Window.Display from event polling
Post by: T.T.H. on February 04, 2008, 08:31:46 pm
Quote from: "l0calh05t"
EDIT:
And why would event handling and graphics both have to access gui data?

E.x. "is a mouse move or mouse click event inside a certain GUI element?" in the event thread and "draw a certain GUI element" in the graphics thread - both would be accessing some rectangle (x, y, width, height) which would then have to be made threads safe with a mutex which would then lower performance due to thread synchronization.

Please note that this only is some naive thinking from me - I've never really used a GUI system.
Title: Separate Window.Display from event polling
Post by: l0calh05t on February 04, 2008, 09:35:53 pm
Quote from: "T.T.H."
E.x. "is a mouse move or mouse click event inside a certain GUI element?" in the event thread and "draw a certain GUI element" in the graphics thread - both would be accessing some rectangle (x, y, width, height) which would then have to be made threads safe with a mutex which would then lower performance due to thread synchronization.

Please note that this only is some naive thinking from me - I've never really used a GUI system.


Some things might be shared (hard to avoid, but should be minimized as much as possible), but in this case for example, both accesses are read accesses, so they can be done in parallel. And for write accesses (which should never be caused by the renderer thread) we'd either have to lock, or find a lockless solution. Or use a pipeline approach. Or double buffer the GUI data. There are many solutions to these kind of problems, the hard part is figuring out which one's the best.
Title: Latest Head Revision
Post by: Joe Somebody on February 25, 2008, 05:17:03 pm
Hi Laurent.  For I pulled down the latest from svn this weekend.   I noticed in the OpenGL Demo (I'm running win32, in Vista x64), that with the latest, whenever there are events coming in, such as the mouse moving, that the cube stops spinning.

This does not happen with the Release 1.2, the cube spins continuously on this version.

It seems like the changes made to the event handling are likely causing this behavior.  Does the sample program have to be updated in order to properly handle the new separate Render/Event system?


Quote from: "Laurent"
I've just finished the modifications.

The event handling is now automatically independant from Display() (you don't have to do anything).

The window's context can be unbound with SetActive(false) (SetActive(true) replaces SetCurrent).

You can get the modifications with SVN.
Title: Separate Window.Display from event polling
Post by: Laurent on February 26, 2008, 02:12:05 am
No, the OpenGL sample (and the code in general) shouldn't be affected by this modification. I've never noticed what you describe, but I'll do more tests to make sure.

Thanks for your feedback.
Title: Separate Window.Display from event polling
Post by: Laurent on February 27, 2008, 04:06:50 pm
I've tested the OpenGL sample again (WinXP 32 bits), there's no problem with events. The code hasn't changed much, I can't see any reason why this issue appears now on your system. I don't even know if it has something to do with Vista or 64 bits.

Did you clean / rebuild all SFML libs and samples, just to be sure ?
Title: Separate Window.Display from event polling
Post by: Joe Somebody on February 27, 2008, 06:36:06 pm
Quote from: "Laurent"
I've tested the OpenGL sample again (WinXP 32 bits), there's no problem with events. The code hasn't changed much, I can't see any reason why this issue appears now on your system. I don't even know if it has something to do with Vista or 64 bits.

Did you clean / rebuild all SFML libs and samples, just to be sure ?


The code was pulled down into a fresh directory, from SVN, then built, so it had to be clean, right?

I copied the executable, etc on to a Windows XP system, but it won't run.  I get an error "This application has failed to start because the application configuration is incorrect.  Reinstalling the application may fix this problem."

The code was built using Visual Studio Express Edition 9.0 for C++.   It definitely is set to have a win32 target, so it has me scratching my head.  I will let you know when I figure it out.