[haiku-gsoc] Re: SDL Patch 01
- From: Stephan Aßmus <superstippi@xxxxxx>
- To: haiku-gsoc@xxxxxxxxxxxxx
- Date: Mon, 27 Jun 2011 10:00:24 +0200
Hi Nathan,
On 27.06.2011 04:27, Nathan Heisey wrote:
Attached is the most recent patch.
Thanks for your work!
Some comments and thoughts on the patch below:
virtual void Minimize(bool minimize)
{
/* This is only called when mimimized, not when restored */
- //SDL_PrivateAppActive(minimize, SDL_APPACTIVE);
+ SDL_SendWindowEvent(window, SDL_WINDOWEVENT_MINIMIZED, 0, 0);
BWindow::Minimize(minimize);
}
Looking at the code below, you should rewrite this as:
if( minimize )
SDL_SendWindowEvent(window, SDL_WINDOWEVENT_MINIMIZED, 0, 0);
else
SDL_SendWindowEvent(window, SDL_WINDOWEVENT_RESTORED, 0, 0);
virtual void WindowActivated(bool active)
{
- SDL_PrivateAppActive(active, SDL_APPINPUTFOCUS);
+// SDL_PrivateAppActive(active, SDL_APPINPUTFOCUS);
+ if( active ) {
+ SDL_SendWindowEvent(window, SDL_WINDOWEVENT_SHOWN, 0, 0);
+ SDL_SendWindowEvent(window, SDL_WINDOWEVENT_RESTORED, 0, 0);
+ } else {
+ SDL_SendWindowEvent(window, SDL_WINDOWEVENT_MINIMIZED, 0, 0);
+ SDL_SendWindowEvent(window, SDL_WINDOWEVENT_HIDDEN, 0, 0);
+ }
}
The above looks misplaced. All these events look like they should be
send from the respective BWindow hooks. You can override Show(), Hide(),
and you already override Minimized(), but need to handle the minimize
parameter as shown. WindowActivated() is called when the window gains or
looses keyboard focus. It has nothing to do with the events you generate.
From your patch, I don't know how SDL_SendWindowEvent() actually works.
Is this an SDL provided function? If so, what about threading? I know
nothing about the SDL port, but I am wondering what thread runs the SDL
event loop? If main() eventually runs the SDL event loop, then it's the
BApplication thread. These hook methods however are run in the BWindow
thread, and it looks very bad to me to just call into SDL functions that
are probably expected to run in the main (BApplication thread). This
gets even worse when there is more than one window on the screen
belonging to the same SDL app. Since the SDL_SendWindowEvent() method
takes the window as parameter, I am assuming this might be the case for
some SDL apps.
You need a strategy to deal with this. If an SDL app can have more than
one window, then there is really only one solid solution, which is to
first dispatch the events to the BApplication looper (via BMessages),
and from the BApplication event loop (MessageReceived()), call into the
SDL functions. This will resolve all threading issues, unless you don't
do it consistently for any and all events that are reported in the
BWindow thread (i.e. also in the hooks of the BView which is added to
that window). Just forward everything as BMessage to the application,
handle it there and call SDL functions from nowhere else. This is
guaranteed to solve threading problems. Your only other option is to add
a global lock which you *always* grab before calling into any and all
SDL functions. In that case you need to be sure you never try to obtain
other locks in a nested fashion, like you should never try to grab the
BWindow lock from your BApplication thread while already holding the SDL
global lock... the messaging solution completely works around such
issues. I've used that approach in the WebKit port.
Best regards,
-Stephan
Other related posts:
