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

Add support for external runloop for main callback API #8946

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tokyovigilante
Copy link

@tokyovigilante tokyovigilante commented Jan 29, 2024

Description

This extends the Main Callback API to prevent event loop iteration internally in favour of allowing an external runloop to track display or other timer-based callbacks and call SDL_IterateMainCallbacks to process SDL events and drive a renderer.

This is most applicable to Wayland-based applications already using the wl_surface_frame callback to trigger another frame to be rendered when requested by the compositor, but is likely to be applicable to other systems where the main runloop is not (or cannot) be controlled by SDL.

The only additional application requirement once the instructions in docs/README-main-functions.md have been followed is to call SDL_IterateMainCallbacks(SDL_TRUE) from a loop or timer function in your code, or in response to system events, e.g. activity on a Wayland display FD.

I am looking at future work to also allow slower/pause screen updates if there are no events or updates to render (i.e. an inactive or static UI), and would appreciate feedback on this or any other aspects of this PR.

Changes

  • Make SDL_IterateMainCallbacks public API
  • Add boolean SDL_HINT_MAIN_CALLBACK_EXTERNAL_RUNLOOP hint
  • exit early from SDL_InitMainCallbacks when external runloop hint set without iterating internally to allow control by external runloop

@slouken slouken added this to the 3.2.0 milestone Jan 29, 2024
Copy link
Contributor

@Kontrabant Kontrabant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine to me as it's intended for applications using the frame callback or some other timing event on their own, and is platform agnostic.

I wouldn't want to internally tie the SDL main callback to the Wayland frame callback though, as that callback is notoriously unreliable as the timing can arbitrarily change, it can just never come, etc… If some application understands the risks and wants to use it on their own though, that's fine.

@icculus
Copy link
Collaborator

icculus commented Jan 29, 2024

Ugh, I want to say no to this, but it seems pretty harmless, beyond locking an implementation detail into the public API.

Let me think about it a bit.

@icculus icculus self-assigned this Jan 29, 2024
@tokyovigilante
Copy link
Author

Thanks both for the feedback.

I understand the reluctance, but although I'm using Wayland, my real motivation for this is to be able to develop in Swift and use the Swift RunLoop mechanism (with GCD) to drive my app (it may also be running as a daemon or minimized with no GUI, or as a client only driving another instance over a network etc) and I couldn't find another way to have SDL integrate with an external runloop (i.e, no exposed FDs to poll on etc).

In theory I could use a while(1) loop to drive my a CFRunLoop in my specific use-case but 90% of my app doesn't use SDL (in fact driven by an audio hardware callback) and would take quite a lot of rewriting.

Regarding Wayland, I haven't found the frame callback to be too unreliable as long as the surface is on-screen, but this is with Sway and other compositors may work differently (i.e. the WAYLAND PROBLEM ;)).

Anyway have tried to make as few SDL changes as little as possible to avoid bringing these issues into the library and as noted, leave it up to the application to opt into.

@icculus
Copy link
Collaborator

icculus commented Jan 30, 2024

Honestly, why not, let's do this. The docs are pretty clear you're on your own if it breaks.

The question, though: should this use a hint, or should this just be a preprocessor thing? Is there a time when a single build might want to do this but sometimes not, so having a variable hint is useful?

If it's something a build will always do, I'd favor doing something like:

#define SDL_MAIN_CALLBACKS_MANUAL_ITERATION
#include <SDL3/SDL_main.h>

(Or whatever we call that.)

@icculus
Copy link
Collaborator

icculus commented Jan 30, 2024

Oh, obvious question I just thought of: this only does something on the generic implementation; what should this do on iOS and Emscripten, where we don't use that code but the app will still be trying to manually manage the iteration?

Maybe this isn't something we should do after all.

@tokyovigilante
Copy link
Author

I don't have a problem with a #define rather than a hint, the hint was just the first thing I tried and worked fine. You're right I can't imagine a use case for changing this at runtime.

I modelled these changes on the iOS code, which does exactly the same thing but just manages the CADisplayLink inside SDL. I would say that if this was to be adopted, the iOS specific code including the CADisplayLink handling could be removed from SDL and made the responsibility of the app.

The current code makes iOS a special case, which I'm not sure is what is really wanted? Continuing that approach would suggest a Wayland-specific implementation using the wl_surface_frame callback as should also be created, i.e either include every special case, or none.

I can't comment on the Emscripten requirements as I have no experience with that platform sorry. I would assume if you don't manually enable this then it would continue to work however it does now? It is intended to be a purely additive and opt-in system, as with the callback API itself.

Let me know what you settle on and I am happy to change the PR to use a #define if wanted.

@tokyovigilante
Copy link
Author

Hi, I have rebased this against main and would appreciate if it could be re-reviewed. Have considered putting this behind a #define rather than a hint, but the support would still need to be compiled into the library for distribution, which makes it harder to alter SDL_InitMainCallbacks() to exit early.

It also may be useful to choose behaviour at runtime, if a GUI is only displayed some of the time, and it does somewhat ease crossplatform-development to use a hint conditionally per-platform rather than #defines. For example Swift doesn't support the C preprocessor other than in library excludes, i.e. could not conditionally enable this with a #define in application code).

Open to any suggestions on how I could do this easily with a #define, but IMO the hint is fairly clean (ie additive only) and clearly "opt-in".

This extends the Main Callback API to prevent event loop iteration internally in favour of allowing an external runloop to track display or other timer-based callbacks and call SDL_IterateMainCallbacks to process SDL events and driver a renderer.

This is most applicable to Wayland-based applications already using the `wl_surface_frame` callback to trigger another frame to be rendered when requested by the compositor, but is likely to be applicable to other systems where the main runloop is not or cannot be controlled by SDL.

The only additional application requirement once the instructions in `docs/README-main-functions.md` have been followed is to call `SDL_IterateMainCallbacks(SDL_TRUE)` from a loop or timer function in your code, or in response to system events, e.g. activity on a Wayland display FD.

[Changes]
* Make SDL_IterateMainCallbacks public API
* Add boolean SDL_HINT_MAIN_CALLBACK_EXTERNAL_RUNLOOP hint
* exit early from SDL_InitMainCallbacks when external runloop hint set without iterating internally to allow control by external runloop
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.

None yet

4 participants