Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

main: Added _optional_ callback entry points. #8247

Merged
merged 12 commits into from
Nov 1, 2023

Conversation

icculus
Copy link
Collaborator

@icculus icculus commented Sep 14, 2023

EDIT: updated this on 10/29/2023 with SDL_AppEvent details, etc.

This lets apps optionally have a handful of callbacks for their entry points instead of a single main function. If used, the actual main/SDL_main/whatever entry point will be implemented in the single-header library SDL_main.h and the app will implement four separate functions:

First:

int SDL_AppInit(int argc, char **argv);

This will be called once before anything else. argc/argv work like they always do. If this returns 0, the app runs. If it returns < 0, the app calls SDL_AppQuit and terminates with an exit code that reports an error to the platform. If it returns > 0, the app calls SDL_AppQuit and terminates with an exit code that reports success to the platform. This function should not go into an infinite mainloop; it should do any one-time startup it requires and then return.

Then:

int SDL_AppIterate(void);

This is called over and over, possibly at the refresh rate of the display or some other metric that the platform dictates. This is where the heart of your app runs. It should return as quickly as reasonably possible, but it's not a "run one memcpy and that's all the time you have" sort of thing. The app should do any game updates, and render a frame of video. If it returns < 0, SDL will call SDL_AppQuit and terminate the process with an exit code that reports an error to the platform. If it returns > 0, the app calls SDL_AppQuit and terminates with an exit code that reports success to the platform. If it returns 0, then SDL_AppIterate will be called again at some regular frequency. The platform may choose to run this more or less (perhaps less in the background, etc), or it might just call this function in a loop as fast as possible. You do not check the event queue in this function (SDL_AppEvent exists for that).

Next:

int SDL_AppEvent(const SDL_Event *event);

This will be called once for each event pushed into the SDL queue. This may be called from any thread, and possibly in parallel to SDL_AppIterate. The fields in event do not need to be free'd (as you would normally need to do for SDL_EVENT_DROP_FILE, etc), and your app should not call SDL_PollEvent, SDL_PumpEvent, etc, as SDL will manage this for you. Return values are the same as from SDL_AppIterate(), so you can terminate in response to SDL_EVENT_QUIT, etc.

Finally:

void SDL_AppQuit(void);

This is called once before terminating the app--assuming the app isn't being forcibly killed or crashed--as a last chance to clean up. After this returns, SDL will call SDL_Quit so the app doesn't have to (but it's safe for the app to call it, too). Process termination proceeds as if the app returned normally from main(), so atexit handles will run, if your platform supports that.

The app does not implement SDL_main if using this. To turn this on, define SDL_MAIN_USE_CALLBACKS before including SDL_main.h. Defines like SDL_MAIN_HANDLED and SDL_MAIN_NOIMPL are also respected for callbacks, if the app wants to do some sort of magic main implementation thing.

In theory, on most platforms these can be implemented in the app itself, but this saves some #ifdefs in the app and lets everyone struggle less against some platforms, and might be more efficient in the long run, too.

On some platforms, it's possible this is the only reasonable way to go, but we haven't actually hit one that 100% requires it yet (but we will, if we want to write a RetroArch backend, for example).

Using the callback entry points works on every platform, because on platforms that don't require them, we can fake them with a simple loop in an internal implementation of the usual SDL_main.

The primary way we expect people to write SDL apps is with SDL_main, and this is not intended to replace it. If the app chooses to use this, it just removes some platform-specific details they might have to otherwise manage, and maybe removes a barrier to entry on some future platform.

This PR updates a few SDL test apps (loopwave, testaudio, testsprite, and testaudiocapture) to use this; it nicely cleans up some ugliness! This doesn't change all of the test apps, because we'd like to exercise both SDL_main approaches, and also, we don't want to give the idea that this is the only way to write an SDL app.

Fixes #6785.

@icculus icculus added this to the 3.2.0 milestone Sep 14, 2023
@slouken
Copy link
Collaborator

slouken commented Sep 14, 2023

I would suggest having the app fill out a structure of callbacks for various app functions. One big thing that's missing here is event handling, and an important one for iOS. It might be nice to have individual callbacks for different events, or maybe just one callback that's passed an SDL_Event * might be enough.

One advantage of using an event callback is that we can manage the "pump, peep, dispatch" loop for them instead of having applications use the while (pollevent) ... model, which can starve if you're throwing enough events at them, or another thread is generating events coming off of the network at a high rate, etc.

@icculus
Copy link
Collaborator Author

icculus commented Sep 15, 2023

It might be nice to have individual callbacks for different events, or maybe just one callback that's passed an SDL_Event * might be enough.

That'd be a long list of callbacks, if we do individual callbacks, and would need updating as we add new features ... I'd rather they just get a callback that provides an SDL_Event in this case...but then the app could also just add an event watcher function in this case, which would trigger outside of the mainloop iteration, as iOS calls the Objective-C event handler hooks.

But most apps would be fine if the platform fills in the event queue elsewhere and then runs through it in their Iterate function; this is more or less how it worked in SDL2.

which can starve if you're throwing enough events at them

Doesn't the sentinel event work solve this?

@slouken
Copy link
Collaborator

slouken commented Sep 15, 2023

which can starve if you're throwing enough events at them

Doesn't the sentinel event work solve this?

Oh yeah, it does. :)

@icculus
Copy link
Collaborator Author

icculus commented Sep 18, 2023

Okay, iOS is in, using a displaylink to trigger the mainloop iterations. Right now this lives on its own CADisplayLink separate from the window, but I might change this to hook into the current key window (or whatever UIKit calls that) once one is created and fall back to some basic timer callback when one isn't available, which would let me take my hack out of SDL_iPhoneSetAnimationCallback.

In theory, macOS could benefit from this approach too, but for now I'm leaving it alone.

The remaining question is if Android has a similar mechanism we could use that would bring any value (and/or that the system would rather we use in any case). @1bsyl, any info you have on this would be helpful. If there isn't an Android-specific mechanism we should use here, it can just use the existing generic mainloop implementation without any changes, as it's just a standard SDL_main function internally.

@1bsyl
Copy link
Contributor

1bsyl commented Sep 19, 2023

@icculus on android, there is the choreographer:

https://developer.android.com/reference/android/view/Choreographer

Coordinates the timing of animations, input and drawing.
The choreographer receives timing pulses (such as vertical synchronization) from the display subsystem then schedules work to occur as part of rendering the next display frame.

with a callback when a new display frame is being rendered

not sure if this is really relevant. because, when apps goes in background / pause/ start / stop, there will be no more iteration calls.
also, the callback is in the SDLActivity (eg activity thread).
where are SDL runs in a thread aside!

@icculus
Copy link
Collaborator Author

icculus commented Sep 19, 2023

Yeah, I think we'll leave Android alone for now. It doesn't seem like it offers any benefits vs just using vsync.

@icculus icculus force-pushed the sdl3-main-callbacks branch 3 times, most recently from f6f62a1 to ce2e0ec Compare September 23, 2023 13:08
@icculus icculus marked this pull request as ready for review September 23, 2023 14:58
@icculus
Copy link
Collaborator Author

icculus commented Sep 23, 2023

Okay, this is good enough to be merged. I'm hesitant to convert all the test apps over to the new mode, lest people think this is the primary way to write SDL programs, but it's very tempting to be able to delete all the Emscripten #ifdefs in there.

So if we want different names for the entry points, or whatnot, speak now!

@icculus
Copy link
Collaborator Author

icculus commented Sep 27, 2023

Last call before I merge this!

@slouken
Copy link
Collaborator

slouken commented Sep 27, 2023

I'm still unsure that we want this. Sell it to me? :)

@1bsyl
Copy link
Contributor

1bsyl commented Sep 27, 2023

This sounds interesting to me ! Implementing the callbacks means that would work on all platforms, whereas main(), isn't compatible with emscripten().
OTH, people are probably more familiar with the usual main(). (not an issue, since that's still possible to use the main()).
OTH not sure how popular emscriptem is.
are there other platform that really require an iterate cb?

The issue I had to convert a game to emscripten was to move the program from the traditional main() function to the required iterate() function, and more precisely, the hard part was the way to handle nested dialog windows/menus in a simple way.

main() had nested event loop (one level per dialog menu) in a recursive way, and it was not really obvious to convert to an iterate() function.

I wonder if SDL_AppIterate() could somehow ease this process.
but not sure how, maybe:

  • a context parameter ?
  • a way to push/pop a new iterate() functions (+context?) ?
  • some parameter for recursive return code ?
    • is the current iterate function() failing
    • or returning to upper level
  • other..
    again, really not sure if it even makes sense. but if there is a clean way, to create navigation menu, that would be cool.

@quyykk
Copy link
Contributor

quyykk commented Sep 27, 2023

I'm still unsure that we want this. Sell it to me? :)

I heard it could solve #1059 👀

@slouken
Copy link
Collaborator

slouken commented Sep 27, 2023

I'm still unsure that we want this. Sell it to me? :)

I heard it could solve #1059 👀

No, it won't.

@icculus
Copy link
Collaborator Author

icculus commented Sep 28, 2023

I'm still unsure that we want this. Sell it to me? :)

Sure!

  • If you've got an app that runs on Emscripten, it lets you remove ifdefs and platform-specific junk from your program.

  • If you're on iOS, you can operate from the animation callback without having to do things that look weird, like returning from main() to get the actual work of the program to start (and again, no ifdefs for iOS in your app to get this behavior).

  • if/when we write a RetroArch backend for SDL, this will be the only reasonable way to implement it, as RetroArch cores--which are plugins, not apps--don't have a main function at all and require an animation callback to render. This was a showstopper for targeting RetroArch in SDL2.

  • If you hate this style and only ever want to use a Unix style main, you can. It's still the way most SDL things are built, and this feature evaporates out through the preprocessor if you don't explicitly request it.

...But grep for EMSCRIPTEN in the test directory, and you can see all the crud that could be cleaned away by this.

@icculus
Copy link
Collaborator Author

icculus commented Sep 28, 2023

I heard it could solve #1059 👀

It would need to be significantly more invasive to SDL's internals than this PR actually turned out to be.

@slouken
Copy link
Collaborator

slouken commented Sep 28, 2023

I still think running the frame loop for them and calling back into an application event callback is going to be a key part of this model.

@icculus
Copy link
Collaborator Author

icculus commented Sep 28, 2023

I still think running the frame loop for them and calling back into an application event callback is going to be a key part of this model.

That would maybe make #1059 easier to solve...? But otherwise, there is already a mechanism for this in SDL_AddEventWatch, if they don't want to just pick up any events we saw since the last run of the main loop from SDL_PollEvent.

Maybe I'm not seeing the bigger picture, though.

@slouken
Copy link
Collaborator

slouken commented Sep 28, 2023

I think it's more about what people would expect from a callback based framework, not what's actually necessary to create an app.

@icculus
Copy link
Collaborator Author

icculus commented Sep 28, 2023

Hmmm, even if we use SDL_AddEventWatch internally, this would make the "you have to respond to SDL_EVENT_DID_ENTER_BACKGROUND right away" code way less awkward for the app...

@slouken
Copy link
Collaborator

slouken commented Sep 28, 2023

Hmmm, even if we use SDL_AddEventWatch internally, this would make the "you have to respond to SDL_EVENT_DID_ENTER_BACKGROUND right away" code way less awkward for the app...

True!

@icculus
Copy link
Collaborator Author

icculus commented Oct 29, 2023

Okay, I was a little salty about adding an event callback, but having done the work, it's actually really nice:

  • It tends to simplify an app's SDL_AppIterate a ton and make a nice clean separation of app logic, if the test apps I updated are any indication.
  • Migrating to this from the usual mainloop best practices isn't hard at all (just move your switch statement into a new function, maybe delete the "done" variable from the top of the main function).
  • You don't need to mess with event watchers to catch time-sensitive iOS events, since your callback sees it as soon as SDL does.
  • You don't have to worry about freeing stuff like in SDL_EVENT_DROP_FILE (SDL does it when the event callback returns).

Currently this doesn't promise what thread event callbacks will trigger on, and they may trigger in parallel to SDL_AppIterate (but in practice, this doesn't happen on any platform, discounting the app using SDL_PushEvent itself from a background thread. The only place we might have was audio device disconnects, and these now queue up until SDL_PumpEvents is called). I didn't want to wrap this in a mutex because I don't want these to serialize if they are time-sensitive. Return values from the app callbacks are managed with an atomic int and compare-and-swap, though, so you don't risk the app requesting an exit in an event handler and then another callback racing overwrites it with zero.

Then again, I'm considering forcing SDL_PumpEvents to return immediately if called during SDL_AppIterate, so that the event pump only works when we're in the event handling phase of the app lifecycle, and while the app can call SDL_PollEvent() in this configuration, it just won't ever have new events to report. Still thinking about it, though.

@icculus
Copy link
Collaborator Author

icculus commented Oct 30, 2023

Okay, I think this is ready to merge (again!).

@icculus
Copy link
Collaborator Author

icculus commented Nov 1, 2023

Okay, concerns are resolved; I'm going to squash-and-merge today if there aren't any objections!

@slouken
Copy link
Collaborator

slouken commented Nov 1, 2023

Actually the documentation you wrote at the top of this pull request is really good. I’m on my phone so it’s hard to read a diff, but if it’s similar, that should be great.

@icculus icculus merged commit 9c664b0 into libsdl-org:main Nov 1, 2023
33 checks passed
@icculus icculus deleted the sdl3-main-callbacks branch November 1, 2023 22:40
@icculus
Copy link
Collaborator Author

icculus commented Nov 1, 2023

Okay, this is merged!

I can't believe this went from "I don't think this is solvable at all" to something that doesn't suck. :)

@sezero
Copy link
Contributor

sezero commented Nov 1, 2023

@icculus: Too late to the party, I know, but here goes:

(1) SDL_main() and SDL_main_func are now marked as SDLCALL: Are we sure
about this?

(2) many new procedures are missed in dynapi - running gendynapi.py outputs:

$ PATH=/opt/python37/bin:$PATH ./gendynapi.py
NEW extern SDLMAIN_DECLSPEC int SDLCALL SDL_AppInit(int argc, char *argv[]);
NEW extern SDLMAIN_DECLSPEC int SDLCALL SDL_AppIterate(void);
NEW extern SDLMAIN_DECLSPEC int SDLCALL SDL_AppEvent(const SDL_Event *event);
NEW extern SDLMAIN_DECLSPEC void SDLCALL SDL_AppQuit(void);
NEW extern SDLMAIN_DECLSPEC int SDLCALL SDL_main(int argc, char *argv[]);

Please fix following warning(s):
-------------------------------
  In file SDL_main.h: function SDL_AppEvent() has 0 '\param' but expected 1
  In file SDL_main.h: function SDL_AppEvent() missing '\param event'
  In file SDL_main.h: function SDL_main() has 0 '\param' but expected 2
  In file SDL_main.h: function SDL_main() missing '\param argc'
  In file SDL_main.h: function SDL_main() missing '\param argv'
  In file SDL_main.h: function SDL_AppQuit() has 0 '\returns' but expected 1
  In file SDL_main.h: function SDL_main() has 0 '\returns' but expected 1
  In file SDL_main.h: function SDL_main() has 0 '\since' but expected 1
done!

@icculus
Copy link
Collaborator Author

icculus commented Nov 2, 2023

SDL_main() and SDL_main_func are now marked as SDLCALL: Are we sure
about this?

SDL calls into these functions that exist in the app; even though it's called "main" it's actually a callback in SDL3 that SDL_RunApp calls.

@icculus
Copy link
Collaborator Author

icculus commented Nov 2, 2023

(2) many new procedures are missed in dynapi - running gendynapi.py outputs:

Hmm, they aren't symbols in SDL; this is a bug in gendynapi.py. It skipped SDL_main before because it wasn't SDLCALL, I assume.

icculus added a commit that referenced this pull request Nov 2, 2023
Otherwise SDL_main and SDL_App* functions look like exported symbols instead
of functions the app is meant to implement.

Reference PR #8247.
@icculus
Copy link
Collaborator Author

icculus commented Nov 2, 2023

Hmm, they aren't symbols in SDL; this is a bug in gendynapi.py. It skipped SDL_main before because it wasn't SDLCALL, I assume.

This is fixed in gendynapi.py now.

@slouken
Copy link
Collaborator

slouken commented Nov 2, 2023

Can you take your top level description and turn it into documentation in docs? They way you described how to use the functions was really good, and not captured in the header documentation.

@icculus
Copy link
Collaborator Author

icculus commented Nov 2, 2023

Yeah, I'll do that.

The headers were written to be API reference (which makes it stilted and repetitive), and I'm about to bridge it to the wiki, and the more conversational text can live in README-maincallbacks.md or something.

@slouken
Copy link
Collaborator

slouken commented Nov 2, 2023

Maybe it can be more general (overview) documentation about using SDL_main.h?

icculus added a commit that referenced this pull request Nov 3, 2023
@icculus
Copy link
Collaborator Author

icculus commented Nov 3, 2023

Maybe it can be more general (overview) documentation about using SDL_main.h?

I took a first shot at this in 853c28e, which is easily editable at

https://wiki.libsdl.org/SDL3/README/main-functions

@slouken
Copy link
Collaborator

slouken commented Nov 3, 2023

Cool, I added questions. Also, can we reference this at the top of SDL_main.h and somehow bridge that to the wiki so people know where to go for more information?

The historical stuff may not be necessary in the new README. Migration information should be in README-migration.md and while I think it's cool to know how things have changed, people probably don't need to wade through an explanation of how SDL 1.2 worked. :)

@icculus
Copy link
Collaborator Author

icculus commented Nov 4, 2023

Okay, updated this in a19029e!

@phosit
Copy link

phosit commented Jan 3, 2024

@1bsyl

a context parameter ?

Also without a context parameter users are forced to use globals to share data between callbacks.

@ahcox
Copy link

ahcox commented Apr 23, 2024

I can confirm this as working on Ubuntu 23.10 with the example below opening a blank window and printing out events.

#define SDL_MAIN_USE_CALLBACKS
#include <SDL3/SDL_main.h>
#include <SDL3/SDL_init.h>
#include <iostream>

namespace {
    /// The window we'll open to show our rendering inside.
    SDL_Window* window {nullptr};
}

extern "C" {

int SDL_AppInit(void **appstate, int argc, char **argv)
{
    std::cerr << "SDL_AppInit" << std::endl;

    int result = 0;
    if(result = SDL_InitSubSystem(SDL_INIT_VIDEO | SDL_INIT_EVENTS) < 0)
    {
        std::cerr << "SDL_InitSubSystem failed with code " << result << std::endl;
        goto error_exit;
    }

    window = SDL_CreateWindow( "SDL3 Window", 960, 540, 0 /* | SDL_WINDOW_VULKAN*/ );
    if( window == NULL )
    {
        std::cerr << "SDL_CreateWindow failed" << std::endl;
        goto error_exit;
    }

    return 0;

    error_exit:
    std::cerr << "Last SDL error: " << SDL_GetError() << std::endl;
    return -1;
}

int SDL_AppIterate(void *appstate)
{
    static thread_local int i = 0;
    ++i;
    return 0;
}

int SDL_AppEvent(void *appstate, const SDL_Event *event)
{
    std::cerr << "SDL_AppEvent";
    if(event)
    {
        std::cerr << ": type = " << event->type << ", timestamp = " << event->common.timestamp << std::endl;
    }
    std::cerr << std::endl;
    if(event->type == SDL_EVENT_QUIT)
    {
        std::cerr << "SDL_EVENT_QUIT" << std::endl;
        return 1;
    }
    return 0;
}

void SDL_AppQuit(void *appstate)
{
    std::cerr << "SDL_AppQuit" << std::endl;
    return;
}

}

@icculus
Copy link
Collaborator Author

icculus commented Apr 23, 2024

  SDL_SetEventEnabled(SDL_EVENT_TERMINATING, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_DID_ENTER_BACKGROUND, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_DID_ENTER_FOREGROUND, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_KEY_DOWN, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_KEY_UP, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_MOUSE_MOTION, SDL_TRUE);

These are all enabled by default, you can just delete these lines.

@ahcox
Copy link

ahcox commented Apr 23, 2024

  SDL_SetEventEnabled(SDL_EVENT_TERMINATING, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_DID_ENTER_BACKGROUND, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_DID_ENTER_FOREGROUND, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_KEY_DOWN, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_KEY_UP, SDL_TRUE);
  SDL_SetEventEnabled(SDL_EVENT_MOUSE_MOTION, SDL_TRUE);

These are all enabled by default, you can just delete these lines.

Thank you, confirmed and deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mainloop control inversion
8 participants