2008-04-24

Modal Gadgets Revisited

Took another look at the Woopsi code today and worked out a way to add modal gadgets into the system without too much effort. You can now make a gadget modal by calling its “goModal()” method, and you can make it stop being modal by calling “stopModal()”. Gadgets automatically cease to be modal when they are closed, hidden or shelved.

There is an important caveat here. In order to achieve this, the goModal() function actually implements its own run loop. A modal gadget steals control of the DS away from the Woopsi instance and assumes control itself. This means that your call to stopModal(), assuming you’re not just waiting for the gadget to close, needs to be included in code that is triggered by one of the modal gadget’s events. There’s no point in including it elsewhere because it’ll never get executed.

Since this functionality is implemented in the Gadget class, all Woopsi gadgets can now be modal. It doesn’t make much sense to make buttons modal, though. Also, you can stack up modal gadgets. If a modal window opens another modal window, when the latest one closes control will return to the previous window. When that closes, control will return to Woopsi. (It’s brilliant when desired functionality just arises out of nowhere - the nature of computer design gives us this stacking with no effort at all.) I’ve removed the ModalScreen class as it is now redundant.

The next alteration is rather complex. There’s a huge change in the way that Woopsi applications should be put together. Until today, the best way to put an app together was to have a main() function that looked like this:


int main() {
    woopsiApplication = new Woopsi();

    AmigaScreen* screen = new Screen("My Screen");
    woopsiApplication->addGadget(screen);

    woopsiApplication->draw();

    while (1) {
        woopsiApplication->run();
        woopsiWaitVBL();
    }

    delete woopsiApplication;

    return 0;
}

Following Jeff’s advice to tidy this up, this is no longer the approved way to do things. The suggested approach is now to:

  • Create a new class and inherit from Woopsi (this will represent your application)
  • Override the startup() function and add all of your GUI creation
  • Use a very simple main() function that just runs the application class

It’s actually simpler than it sounds. If we want to use the new approach with the above main() function, we would create a new class like this:


#ifndef _MYAPP_H_
#define _MYAPP_H_

#include "woopsi.h"
#include "amigascreen.h"

class MyApp : public Woopsi {
public:
    void inline startup() {
        AmigaScreen* screen = new Screen("My Screen");
        woopsiApplication->addGadget(screen);
    }
};

#endif

It’s a very simple class that inherits from Woopsi and adds a screen in its startup() method. In addition to this, we need to modify the main() function in main.cpp:


int main(int argc, char* argv[]) {

        // Create the application
    MyApp app;

        // Run the application
    app.main(argc, argv);

    return 0;
}

The main() function now only consists of three lines. We just create a new instance of our MyApp class and call its main() method. All of the other code, such as the while loop and the initial draw, is now handled by Woopsi. As a nice side-effect, drawing is now disabled until the entire GUI is set up. Gadgets will no longer get drawn gradually as they are being created.

This new main function obviously affects SDL users. Things have improved here, too. The SDL run loop has been merged into the Woopsi code along with the standard DS run loop (it is #ifndef’d out), which means it’s no longer necessary to have a large block of custom code in main.cpp when compiling for SDL. The main() function does need one minor adjustment, but this can be left in place even when compiling for the DS. The SDL version of the main() function above would look like this:


int main(int argc, char* argv[]) {

        // Create the application
    MyApp app;

        // Run the application
    app.main(argc, argv);

#ifdef USING_SDL

    // Shut down SDL systems
    SDL_Quit();

#endif

    return 0;
}

The SDL version of woopsifuncs.h has the USING_SDL define set, so this code is ignored unless you are compiling an SDL version of a Woopsi application.

The demo code uses this new structure should you need further examples of how to re-arrange your programs.

There are a couple of other changes. Buttons now change colour when they are clicked. I fired up WinUAE and noticed that Amiga buttons did this, and they looked a lot better for it.

I’ve also added a “CycleButton” class. The cycle gadget was the Amiga’s equivalent of a drop-down list. I don’t think drop-downs had been invented when the Amiga UI was created, so the programmers implemented a button that cycled through a set of options when clicked, changing its value.

Drop-downs are going to be difficult to implement for two main reasons - one is the limited space on the DS, and the other is the fact that child gadgets cannot be larger than their parents. Well, they can, but any extra area is clipped out and not drawn. Drop-down lists that were larger than the area of their containing window would, therefore, have to follow a similar approach to the context menu (ie. a gadget that exists in the Woopsi object as a peer of the screen objects) and it gets complicated. I’d probably re-use the context menu if I did implement drop-downs, which would work but would be something of a bodge. I think the hacks that added drop-downs to the Amiga followed the same approach.

So, cycle gadgets it is. Along with the (still in-progress list box) I think that covers all possibilities anyway.

One last change. I’ve made a few minor changes to the logo. Looks a little cleaner now.

Woopsi Logo

Comments

Jeff on 2008-04-24 at 23:16 said:

A few niggles.

Why does source/main.cpp #include “scrollingtextbox.h” ?

Why doesn’t the SDL_Quit() go into Woopsi::main() so programmers don’t need to worry about it?

One trick, which I’m not completely proud of, is that I make my conditional code based on _SDL_H (which is #define’d by . Then, in XCODE, I add sdl.h to the precompiled headers. This requires zero change to your source to build for OSX/SDL

Having said that, I see that Woopsi uses USING_PALIB for PAlib so I guess its a standard you’ve arrived at?

I fully agree on the drop-down, I was thinking that that sort of button was exactly what the ContextMenu support was for.

Jeff on 2008-04-24 at 23:17 said:

… _SDL_H is defined by <sdl.h>

Jeff on 2008-04-24 at 23:27 said:

Hmmm, the absolute purist in me thinks it should be:


int main(int argc, char* argv[]) {

        // Create the application
        MyApp app;

        // Run the application
        return app.main(argc, argv);
}

which makes your SDL_Quit() thing a bit harder. But its not as if main() returns to anywhere… yet. Who knows what the libnds/devkitARM guys might manage to implement in the future?

Jeff on 2008-04-24 at 23:42 said:

Actually, this latest round broke things for me, though its not tragic and I doubt it affected anyone - it was to do with the way I subclassed ::startup()

My HPCalculator::startup() called Woopsi::startup() expecting that that left the drawing environment set up. But it doesn’t any more, that happens after startup() returns.

My ::startup() was the thing that drew the fade-in/fade-out splash screen which made it a bit disconcerting because I got a 10 second black screen followed by a blit-in of the actual calculator.

Still, I’m pretty sure it just means I switch to overriding preLoop() instead of startup()

Jeff on 2008-04-24 at 23:46 said:

Yes, that seemed to fix it, at least in DesMume.

I would point out to people that the latest OSX DesMume is an insane CPU hog. Activity Monitor reports it using about 70% of the CPU when there is no ROM open, and 124% when its actually running. Gag!

Jeff on 2008-04-24 at 23:48 said:

Actually, the 70% looks like a measurement error, or something transient - after a while, it drops down to 3% - but its definitely eats the entire CPU when you are running something…

chango on 2008-04-25 at 00:40 said:

Sigh….. A few days I wrote in the forum a quick hello world following the now old way of doing things… Does this approach affects 0.31 or just SVN?

ant on 2008-04-25 at 05:32 said:

Didn’t know that the SDL define was already in there, but now that I think about it, it’s obvious. I’ll switch that around.

The SDL quit code could go into the Woopsi class. I’d done that initially, but then realised that if someone was using Woopsi as a GUI to launch something else the last thing they’d want was for it to automatically shut down SDL.

Maybe I need to revisit the names of the startup/preloop/etc functions to make it more obvious what they do.

Jeff on 2008-04-25 at 07:06 said:

Fair point on the SDL Quit in non-DS applications, though those people can subclass away main() if they want to, remembering that they have a lot more RAM than me and are less likely to be noobs.

I’ve gone through my various apps and upgraded them to use the new scheme with mostly success - however, I’m still thinking about what needs to happen when. This was the startup() in one of my apps.


//
// called after startup
//
void OrganizerApplication::Startup(void)
{
        // do whatever our superclass wants
        Woopsi::Startup();

        fatInitDefault();

        // then do our stuff
        _aboutScreen = new AboutScreen();
        addGadget(_aboutScreen);
        _aboutScreen->flipToTopScreen();

        _menuScreen = new MenuScreen(this);
        addGadget(_menuScreen);

        draw();

        NDS_WIFI::Startup();
        NDS_WIFI::ConnectToAccessPoint();
}

I don’t claim its right, only that its what I had. I want to get clear in my mind exactly when you think things are going to happen. From my perspective, I think that fatInitDefault() needs to happen pretty early, but not earlier than initWoopsiGfxMode() for obvious irq reasons.

Then I create my two screens (the app will have more, but the initial condition is two. Then Woopsi::draw() to get it onscreen.

Then I do the Wifi connection, which I want to have a ‘progress window’.

I think you expect me to put the initial screen/window/gui creation into Woopsi::startup() because its followed by a draw() which will otherwise display gray screens.

Whilst I could put fatInitDefault() into the Woopsi subclass constructor, startup() will be just the same, and perhaps better because I can throw out a ‘cant init fat’ from startup() whereas its nasty to throw from a constructor, especially of a stack based object.

My Wifi connect would happen in preLoop(), though obviously other apps might wait till the user pressed a button (and mine might as well) - its just an example at this point.

On the other hand, the hp calculator used to fade the splash screens out/in prior to setting up the screens, whereas now your main gets a draw() in between startup() and preLoop() which I think means I flash gray at startup. Need to download and check that…

(This is not your problem, I’m sort of talking out loud to convince myself - I think the answer is that my startup() needs to fade out, and my preloop() fade in - minor tweak to the fade code required)

ant on 2008-04-25 at 08:33 said:

Chango, this only affects the SVN code, and any releases of Woopsi from now on. I’m hoping to get version 0.32 out soon. Unfortunately, that’s one of the problems with using a library that isn’t even in alpha status yet. Things will shift around under your feet. If you find that the current releases do everything you need, you should stick with those; alternatively, if you want to keep upgrading as new versions come out, you’ll have to be prepared to make changes to keep up with the changing library.

There’s just no way I can possibly retain complete backwards compatibility between versions of Woopsi at this stage.

Jeff, I think the problem is, basically, the automated call to draw(). If I strip the main back to the basics and leave responsibility for drawing to the developer, I don’t need to care about trying to second-guess how people will use the system. All I need to do is include a note in the docs that says “call draw() or you won’t see anything” and I solve the problem. The main() function then goes back to this:

startup(); runLoop(); shutdown();

The developer just needs to add these two lines somewhere in their startup code:

enableDrawing(); draw();

Any thoughts?

ant on 2008-04-25 at 08:35 said:

Oh, and I think you’re right on the SDL shutdown code. I can move it into Woopsi and, as long as it’s documented, I don’t think it’ll be a problem.

Jeff on 2008-04-25 at 22:54 said:

Don’t misunderstand me, I’m relatively happy with the current sequence, though the names are less than ideal (but I don’t have ideal names to offer). I really have a problem to which I don’t have a kneejerk answer. In your documentation for each of the ‘main-loop’ methods, etc, you should be describing what can and cannot be done at that particular point.

The irqInit() vs wifi setup is a classic to catch people out, especially since the only info most of them get are the bazillions of slightly-different boilerplates out there in other peoples code. A tiny explanation that says “if you want to do any low-level initialisation, it should be at the START of your startup() method, or is that the END? or at the end of your constructor”. Its the “ors” in the previous sentence that sticks in my throat - which do you think will be appropriate for the future? Thats the crux of the question I’m trying to ask. I know its a tough question, but in the world of the DS, its something that every coder is required to answer (whether they know it or not)

(It might end up being appropriate to add another stub called initHardware() or some such, that makes it clear what you expect to happen in there. And thats where the initWoopsiGfxMode() stuff might end up?)

Similarly, you (that is, the framework designer) expect that people will set up their GUI in the startup method - the fact that I (a framework (ab)user) gets fade-in problems is mostly my problem, that I can fix without issue. (I have already). I just needed to have the rules articulated.

I’ve been writing in my source code comments to the effect that:

// called prior to drawing environment being available MyApp::startup() …

// drawing environment is intact, runloop not started yet MyApp::preLoop() …

// runloop is over, drawing environment still available MyApp::shutdown()

which gives me the answers I need.

There definitely are good reasons for having seperate preLoop() and shutdown() - the only thing that needs to happen is that the difference between those two times be understandable by the person who comes along to use them.

Jeff on 2008-04-25 at 23:04 said:

Chango: As far as I can tell, the old way of doing things will still work - inclusion of main into the Woopsi class hasn’t changed the order that you need to do things. Its only a convenience at this point.

So, doing it yourself isn’t the “approved” way. But it still works. Thats pretty much the standard problem with most of the libnds based apps that I’ve seen - you get to initialise the box in whatever way you like and its your problem to make sure its consistent and sensible.

Its better to get in now, early in the process, and prevent masses of almost-correct-boilerplate code being copied from project to project.

Jeff on 2008-04-26 at 03:00 said:

Damn, the change to ‘main as a method’ doesn’t work for SDL builds, because of the SDL hackery, which #define’s main to SDL_main.

Not quite sure how to move forward on that either because having the hack in goofs up the method definition in woopsi.h

Jeff on 2008-04-26 at 04:56 said:

Hmmm, no, perhaps its just my SDL project - need to go through and set it all up again from scratch, I think.

Jeff on 2008-04-26 at 06:30 said:

Yes, I was caught out because my SDL project pointed to a different copy of all the sources. And when I tried just blind-copying across all the new sources across, of course, I smashed the woopsifuncs.* files.

Jeff on 2008-04-26 at 06:45 said:

Ok, a legitimate SDL problem this time, to do with quitting. When you Quit from the OSX menu, the Woopsi object ends up going through its destructor:


Woopsi::~Woopsi() {
    abortRunLoop();
    deleteSystemFont();
    _font = NULL;
    singleton = NULL;
    _contextMenu = NULL;       // UH OH
}

It then goes through the Gadget destructor which crashes here:


    // Close the context menu if we're closing the gadget that opened it
    if (woopsiApplication->getContextMenu()->getOpener() == this) {
        woopsiApplication->shelveContextMenu();
    }

getContextMenu() is returning NULL which is being dereferenced.

Jeff on 2008-04-26 at 07:23 said:

AArrgghh, its even worse than that. Woopsi’s destructor is nulling the SINGLETON pointer which means that its the dereference of ‘woopsiApplication’ that is crashing. ie, you don’t get the NULL coming back from getContextMenu() because you don’t get that far. So far, the fix looks like:


    // Ensure that the gadget is disposed of correctly if it has not been sent to the deletion queue.
    if (!_flags.deleted) {

        _flags.deleted = true;

        // Prevent gadget from receiving VBLs
        if (woopsiApplication) {
            woopsiApplication->unregisterFromVBL(this);
            unregisterChildrenFromVBL();

            // Close the context menu if we're closing the gadget that opened it
            ContextMenu *menu = woopsiApplication->getContextMenu();
            if (menu && menu->getOpener() == this) {
                woopsiApplication->shelveContextMenu();
            }

            // Divorce child from parent
            if (_parent != NULL) {
                _parent->removeChild(this);
            }
        }
    }

but thats a bit clunky.

Jeff on 2008-04-26 at 08:22 said:

Another SDL gotcha. Calling processOneVBL() in a loop is not the same as runLoop because it doesn’t check for the SDL termination condition. In fact, I’m not sure it handles clicks, etc either.

Just starting to click through it in the debugger now…

Jeff on 2008-04-26 at 08:43 said:

I switched to using goModal() and clicks in the list work again. But when I call stopModal(), sometime later I get a crash in raiseVBLEvent() trying to dereference the eventhandler.

Perhaps its worth noting that since goModal is a gadget function, my FileSelector had to run it against a Screen, whereas it was a List on the screen that was processing VBL’s. I wonder if stopModal() interferes with the Screen’s children - the list unregisters itself in its destructor so it should be intact right up till the Screen is deleted…

ant on 2008-04-26 at 09:04 said:

I’ll try to answer all of the points raised so far…

Lifecycle:

I’ve switched back to just a startup() and a shutdown() method in the Woopsi application lifecycle. I did think of putting in hardware init/shutdown as well as app startup/shutdown methods, but that gets problematic.

For example, the FAT initialisation code can apparently take several (10?) seconds on some flash carts. In that case, it seems best that the GUI starts up and displays an “Initialising FAT” message before that hardware is set up, which gives us this order:

initApp(); initHardware(); run(); shutdownApp(); shutdownHardware();

However, we’re already initialising the IRQs and video hardware, so this pattern is already misleading. Also, people may want to set up the sound hardware before the GUI, so that they can play a startup sound. That would lead to this structure:

preInitHardware(); initApp(); postInitHardware(); run(); shutdownApp(); shutdownHardware();

It’s impossible to cater for every eventuality without a huge number of functions. It’s much easier to provide a basic startup/run/shutdown structure and allow people to call their own extra setup/shutdown functions from the ones provided.

Woopsi destructor bug:

I’ve added your fix. Looks fine to me.

SDL processOneEvent():

I’ve moved the SDL quit check into processOneEvent, so all modal gadgets will listen for SDL quit events. If a quit event occurs, all modal gadgets will stop being modal. I’ve also switched around the Woopsi runLoop() so that it uses the modal system. The Woopsi::main() function calls goModal(), and I’ve removed the runLoop() function.

raiseVBLEvent() crash:

Not sure what’s causing that. Unfortunately Xcode is borked again so I can’t test it here. I’ll have to see if I can get it up and running again before I can do anything else.

EDIT: Xcode problem caused by trying to start it up with an iPhone or iPod Touch already connected. Downloading the latest iPhone SDK to see if the new Xcode fixes the problem.

Jeff on 2008-04-26 at 10:06 said:

It turns out that my list didn’t actually need to be registered for VBL so I removed that and the crash disappeared. I suspect the problem is that you get into a recursive call to Woopsi::processOneVBL(). ie, the top level catches the stylus press, launches the function that creates a new screen, INSTALLS ANOTHER VBL LISTENER, goes modal, process events, DELETES THE NEW LISTENER, gets back to the top-level and dies for some reason.

Doesn’t really make sense unless perhaps there’s a re-entrancy problem in the iterating through the vbl listener list? Which I can’t see…

Anyway, crash has gone away so I’m happy to continue pretending it didn’t happen

:-(

ant on 2008-04-26 at 12:49 said:

Hmm, I can’t replicate this bug… Do you have the original code with the bug?

ant on 2008-04-26 at 21:50 said:

Yeah, it’s a real pain to have to remember which bits to copy and which bits not to. I’ve nearly screwed things up a couple of times copying the wrong files around. This should make things much easier.

I’ve spotted your problem. In the Woopsi class, the VBL listeners are processed before the delete queue. You set your FileSelector as the event handler for all of the gadgets within it. As soon as the FileSelector chooses an option, you delete the FileSelector but you don’t unregister any of the gadgets (naturally assuming that it wouldn’t be a problem). On the next VBL, Woopsi processes the VBL listeners, and the gadgets that used to be in the FileSelector raise the events to the (now deleted) FileSelector object. Instant crash.

I’m moving things around so that the delete queue gets processed before the VBL queue, which fixes the bug.

ant on 2008-04-26 at 21:59 said:

…and deleted gadgets no longer raise events.

Jeff on 2008-04-26 at 22:23 said:

http://rapidshare.com/files/110655203/dsorg270408.zip.html is a fairly large zip file which should be the complete project directory - it contains a copy of the SDL sources (I don’t normally have them installed), the xcode project, etc.

If you run it in XCODE, press Browse, it will show some files, press Cancel and it will get a EXC_BAD_ACCESS in Gadget::raiseVBLEvent() - its hard to discern, but the current object has an _source which means its a List.

If you look in list.cpp and remove the constructor/destructor lines that register/deregister for vbl’s, the problem goes away. Its worth noting that the List class no longer has a vbl() method so its running the default one from Gadget. I used to use it for double-click timing but the vblcount() does that for me now, and I only half cleaned it up.

Jeff on 2008-04-26 at 22:28 said:

The SDL directory has some minor tweaks - I changed Debug so that it just uses fprintf(stderr,…) if its being used in SDL mode.

I think the merging of the two source directories will make a big difference - I spent quite a bit of time in the build/run/debug cycle before I realised that I was running on an old version - I’ve got too used to working from the library these days.

ant on 2008-04-26 at 22:46 said:

It’s actually stranger than that. The gadget destructor does this to delete its children:

for (u8 i = 0; i < _gadgets.size(); i++) { _gadgets[i]->destroy(); }

When it tries to delete your file selector screen, it knows that _gadgets has three items (two buttons and the list). For some reason it only runs this loop twice and does not delete the list. Since the list is not marked as deleted it still tries to fire events and check its parent, but its parent has been deleted.

This makes no sense…

EDIT:

Except I’m calling removeChild(), so each time the loop runs there’s one less gadget in the array. Duh.

ant on 2008-04-26 at 22:58 said:

Fixed!

ant on 2008-04-26 at 23:08 said:

It’s a question of balancing flexibility with hiding as much of the inner workings as possible. In the case of enableDrawing()/draw(), the only ways to achieve the latter were to severely limit the former, or make the API unnecessarily verbose. This seems like the only compromise I can think of that lets me achieve what I need to in the demo app.

I’ve added a couple of “Hello World” demos into the new “examples” folder. One uses the old way of doing things, whilst the other uses the new way. They illustrate (and document) how I imagine people will set up Woopsi apps.

Jeff on 2008-04-27 at 00:02 said:

So the problem was that FileSelector got deleted immediately, but when it closed the Screen it had created, that did not get deleted till after the next VBL cycle - because of the deferred deletion stuff.

That makes sense.

The only gotcha I can think about requiring users to do the enableDrawing()/draw() is that it sort of mandates that they have a subclass of Woopsi, or edit the default main.cpp. Big picture, I don’t think thats a problem but thats me, I can believe there would be other people who didn’t want to. However, subclassing the application class to get features seems like a rational “framework design principle” to me.

Jeff on 2008-04-27 at 00:30 said:

As at version 600 of gadget.cpp, I appear to be working again.

Are you planning on moving the regular demo into the examples directory as well? There’ve definitely been times when people thought that it was the start template…