Change SDL2 backend to use controller events.

[dkosmari]

This changes the SDL2 backend to use SDL_CONTROLLER* events instead of peeking into an array of controllers.
ImGui_ImplSDL2_SetGamepadMode() was removed.

Explanation

Having the backend peek into the controller state from inside ImGui_ImplSDL2_NewFrame() (as opposed to ImGui_ImplSDL2_ProcessEvent()) was causing me trouble when using the on-screen keyboard on the Wii U. Currently, the SDL2 port does not handle the keyboard, so I have to manually feed it input events; and to avoid conflicts with ImGui, any event sent to the on-screen keyboard has to NOT be sent to ImGui, or it starts interpreting dpad up/down (as the user navigates through the keyboard keys) and touch events as an attempt to focus away from an Input* widget. The logic I have to implement is:

• If the on-screen keyboard is visible, input events are sent to the keyboard routines.
• If the on-screen keyboard is not visible, input events are sent to the SDL2 backend.

I can bypass _ProcessEvent() when the on-screen keyboard is visible, but that's pointless when the controllers are being read from within _NewFrame().

Because controller events are now being handled in _ProcessEvent(), there's no use for a controller array to be kept around in the backend. Games/apps will open/close controllers as they're attached/detached, and manage it themselves, possibly wrapping them in C++ classes; so it's obnoxious having to keep an array of controllers just to feed to the SDL2 backend through ImGui_ImplSDL2_SetGamepadMode().

I've updated most of the SDL2 examples, to open/close the controllers as they're attached/detached. I'll wait for your feedback before doing the same for the SDL3 backend and examples.

[PathogenDavid]

Your PR removes ImGui_ImplSDL2_SetGamepadMode, but I'm pretty sure you could've just used ImGui_ImplSDL2_SetGamepadMode with ImGui_ImplSDL2_GamepadMode_Manual to solve your problem by removing the controller from the backend when the on-screen keyboard is open and adding it back when it closes.

This PR also has undesirable behavior changes. In particular: ImGuiBackendFlags_HasGamepad is never cleared when all controllers are removed, the default behavior is no longer AutoFirst, and you can't directly select which controllers Dear ImGui uses at all.

Forcing everyone to manually handle SDL_CONTROLLERDEVICEADDED/REMOVED is also just unnecessarily breaking people IMO.

[dkosmari]

1. No other backend takes responsibility for managing the lifetime of gamepads. I don't think this one should either. Unless you just want to make the examples "simpler."
2. Every app/game that uses gamepads is already opening and closing them in response to the device added/removed events. It's a bit weird to call it "forcing everyone."
3. Regarding my own particular issue, ImGui_ImplSDL2_GamepadMode_Manual requires an array of gamepads. My app does not have an array of gamepads. Having to maintain such an array, and keep it in sync with the backend, is what I'd describe as "being forced to," as it's not as trivial as one function call during initialization.
4. The "auto" mode, which is the default, is sketchy at best, since it goes around closing controllers (during the _NewFrame() call) that I manage in my own application. It's very unfortunate that the default is for the backend to crash applications that are already using gamepads by themselves.
5. I can clear ImGuiBackendFlags_HasGamepad when all controller devices are removed, I just skipped that because I didn't see it being useful.

How about a compromise, with a new mode, ImGui_ImplSDL2_GamepadMode_Events (which I think should be the default)?

[PathogenDavid]

1. No other backend takes responsibility for managing the lifetime of gamepads. I don't think this one should either.

The opening and closing of gamepads is an SDL-exclusive concept. None of the other backends need to think about it in the first place so this isn't really relevant.

2. [...] It's a bit weird to call it "forcing everyone."

I'm not saying it's a huge burden to handle to handle those events, it's about breaking things that were already working all because...some dude had strong opinions about the way it worked before and it wasn't his preferred solution?

3. [...] Having to maintain such an array, and keep it in sync with the backend, is what I'd describe as "being forced to," as it's not as trivial as one function call during initialization.

Again it's more about forcing change than forcing the actual code. Your app has unusual requirements, it's not unreasonable to say the burden for accommodating those unusual requirements should fall on you.*

Either way, you don't need to maintain an array if you really don't want to. You can call ImGui_ImplSDL2_SetGamepadMode(ImGui_ImplSDL2_GamepadMode_Manual, nullptr, 0); to tell the backend to stop processing controllers and then ImGui_ImplSDL2_SetGamepadMode(ImGui_ImplSDL2_GamepadMode_AutoFirst); to turn it back on.

Heck, you could probably even just toggle ImGuiConfigFlags_NavEnableGamepad and avoid touching the backend at all. (That might have some unintended side-effects though.)

(*This isn't to say that Dear ImGui shouldn't handle this better. Maybe this situation exists on other platforms with soft keyboards and it's worth solving generally, but I don't think a general solution would look anything like yours.)

4. [...] It's very unfortunate that the default is for the backend to crash applications that are already using gamepads by themselves.

Controllers in SDL are reference-counted, the backend opening and closing them has no bearing on your controllers.

If you're seeing crashes from this it's a bug in the SDL port you're using.

How about a compromise, with a new mode, ImGui_ImplSDL2_GamepadMode_Events (which I think should be the default)?

This would just make the implementation of the backend unnecessarily convoluted.

[ocornut]

ImGui_ImplSDL2_SetGamepadMode(ImGui_ImplSDL2_GamepadMode_Manual, nullptr, 0); to tell the backend to stop processing controllers and then ImGui_ImplSDL2_SetGamepadMode(ImGui_ImplSDL2_GamepadMode_AutoFirst); to turn it back on.
Heck, you could probably even just toggle ImGuiConfigFlags_NavEnableGamepad and avoid touching the backend at all. (That might have some unintended side-effects though.)

Yes it seems like either of those solutions would work for you and be less intrusive.

A possible third option is we implement ImGuiConfigFlags_NoGamepad (the same way we have ImGuiConfigFlags_NoKeyboard ImGuiConfigFlags_NoMouse) which would be a lower-level filter than e.g. ImGuiConfigFlags_NavEnableGamepad. But ImGui_ImplSDL2_SetGamepadMode(ImGui_ImplSDL2_GamepadMode_Manual, nullptr, 0); will have the same results.

[ocornut]

I believe the underlying issue was that ImGui_ImplSDL2_SetGamepadMode(ImGui_ImplSDL2_GamepadMode_Manual) wouldn't accept an empty array. This is now fixed with 4c2e7bb by rewording the assert.

[ocornut]

As suggested your PR also does too many other things, ones mentioned above, and swapping raw buttons on io.ConfigNavSwapGamepadButtons would essentially end up swapping twice which is incorrect, which suggest the code hasn't been tested much or is part of a larger mod.

My understanding is that you can solve your stated problem by calling ImGui_ImplSDL2_SetGamepadMode(ImGui_ImplSDL2_GamepadMode_Manual, nullptr, 0) when your software keyboard is active. You may also use io.SetAppAcceptingEvents(false) to bypass all events. Please confirm that it solves your problem and that this PR doesn't seem needed anymore. Thanks!

[ocornut]

Daniel could you confirm that my assumption that you can fix your problem entirely with this is correct?
I don't want to ditch a PR if you have an unsolved problem.

[dkosmari]

I could "fix" the problem way before this PR, by just disabling gamepad navigation. If the current design is intentional, there's no reason to change it.

[ocornut]

Disabling keyboard navigation was indeed a possible workaround but it would still mean passing inputs that other key polling systems shouldn't see, so I suppose the new lower-level solution is better. Thanks for your help!