[haiku-gsoc] Re: haiku-gsoc Digest V4 #22
- From: Nathan Heisey <nathanheisey@xxxxxxxxx>
- To: haiku-gsoc@xxxxxxxxxxxxx
- Date: Mon, 27 Jun 2011 23:16:40 -0700
Stephan, Ryan,
Thanks for your advice. Patch 01 was premature; most of the modifications
introduced are going under heavy review and redesign at the moment, based
upon the x11/windows code, as suggested. I will keep the rest of your
advice in mind as well as I become ready to deal with it. Currently I am
writing the functions that generate events; I will redesign the event
handling once I have made significant progress there.
I won't be correcting this patch since I need to rewrite most of that
anyway. The next patch I release will probably contain corrections to the
windowing code; whether it contains your suggested corrections to the event
sending/handling is currently up for grabs. Don't expect it for a while,
though.
I'll read through your advice more carefully and address all of your
questions tomorrow; it's late and I don't want to try and absorb stuff while
I'm nodding off. :)
Thanks for your feedback, it really helps.
-Nathan
On Mon, Jun 27, 2011 at 10:07 PM, FreeLists Mailing List Manager <
ecartis@xxxxxxxxxxxxx> wrote:
> haiku-gsoc Digest Mon, 27 Jun 2011 Volume: 04 Issue: 022
>
> In This Issue:
> [haiku-gsoc] Re: SDL Patch 01
> [haiku-gsoc] Re: SDL Patch 01
>
> ----------------------------------------------------------------------
>
> Date: Mon, 27 Jun 2011 10:00:24 +0200
> From: =?ISO-8859-1?Q?Stephan_A=DFmus?= <superstippi@xxxxxx>
> Subject: [haiku-gsoc] Re: SDL Patch 01
>
> 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
>
> ------------------------------
>
> Date: Mon, 27 Jun 2011 05:21:40 -0400
> From: "Ryan C. Gordon" <icculus@xxxxxxxxxxx>
> Subject: [haiku-gsoc] Re: SDL Patch 01
>
>
> > 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
>
> Stephan's advice is correct.
>
> I'll mention here too that moving the 1.2 code to 1.3 is going to be
> painful as there are almost no 1-to-1 mappings. Use the 1.3 Windows,
> Mac, or X11 code as a template for the SDL API, and the 1.2 BeOS code
> for a starting point on how to talk to the OS, and write it from
> scratch. Your email mentioned a rewrite, so you're probably already on
> this path, but in case you're not, you should seriously consider it.
>
> Some good news, though: the BeOS audio code was already ported to the
> new 1.3 API, so assuming there aren't any Haiku-specific additions to be
> made to it, that part should already work. :) (that's in
> SDL/src/audio/baudio, if anyone is curious. I'm not against renaming
> this "haiku" or something, though!).
>
> --ryan.
>
>
> ------------------------------
>
> End of haiku-gsoc Digest V4 #22
> *******************************
>
>
Other related posts:
