SDL2 backend: Implement multiple gamepad support

[lethal-guitar]

trafficstars

This makes the SDL2 backend work correctly with multiple gamepads. Instead of hard-coding gamepad index 0, we keep a list of all currently available gamepads. When gamepads are added or removed, we update the list accordingly. When processing gamepad input, we then iterate over this list and combine the button and axis state of all gamepads.

Note that it's not necessary to enumerate gamepads during initialization of the backend, because SDL automatically sends a SDL_CONTROLLERDEVICEADDED event for every gamepad that was already present during initialization of SDL.

The main motivation for this change is that I can have multiple gamepads connected to my system and grab any one of them at random, and have ImGui recognize my inputs. Previously, this was not guaranteed, because the gamepad I grabbed might not have index 0. It's also possible now to (for example) use the D-pad on one gamepad and buttons on another one at the same time, although that seems less useful in practice :)

Additionally, we now properly close all previously opened gamepads when shutting down the backend.

[ocornut]

Thanks for the PR.

Some quick feedback:

• please squash into single commit.
• missing break statement in ImGui_ImplSDL2_ProcessEvent()
• ImGuiBackendFlags_HasGamepad is now set in two places.
• why exactly linking with Shell32.lib? and if so probably should be added elsewhere?
• style: single statement scopes don't need braces.
• style: you can use .Size member of ImVector.
• not sure why you introduced new functions ImGui_ImplSDL2_MapAnalog() etc. that code is designed to be dense and out of the way, and is very unfrequently changed.
• you can update the changelog at the top of the imgui_impl_sdl.cpp file.
• can SDL_GameControllerOpen() return NULL and if so what will happen on SDL_GameControllerClose() ?

Thank you

I am not sure this change is desirable for all users but I'm open to try. I imagine backends may need some forms of settings eventually.

[lethal-guitar]

Thanks for the quick feedback! I believe I've addressed most of your comments now.

please squash into single commit.

Done - I still kept the commit adding Shell32.lib separate, as I'm now wondering if that maybe should be its own PR?

So what happened was that I wanted to build the SDL2 example in order to preapre this PR - I had only built ImGui as part of my own project's build system before. But I then got a linker error when running build_win32.bat:

SDL2main.lib(SDL_windows_main.obj) : error LNK2019: unresolved external symbol __imp__CommandLineToArgvW@8 referenced in function _main_getcmdline

I then consulted MSDN for the referenced function CommandLineToArgvW and found that it's located in Shell32.lib, so I added that and the build succeeded.

My suspicion is that earlier versions of SDL (I'm using 2.0.14) did not require this lib, so the build worked fine, but some newer version now needs it. But I need to verify that.

Let me know if you'd prefer a separate PR for that change, as it is indeed unrelated to the rest of this PR.

missing break statement in ImGui_ImplSDL2_ProcessEvent()

Thanks, added!

ImGuiBackendFlags_HasGamepad is now set in two places.

Whoops, forgot to remove the old one, fixed now. Thanks!

style: single statement scopes don't need braces. style: you can use .Size member of ImVector.

Both done ✔️

not sure why you introduced new functions ImGui_ImplSDL2_MapAnalog() etc. that code is designed to be dense and out of the way, and is very unfrequently changed.

I personally find the one-liners harder to read, but your point that this is unlikely to change frequently makes sense. And it also seems better to keep the code local to ImGui_ImplSDL2_UpdateGamepads. So I went back to keeping everything in the macros as one-liners now.

To explain how I ended up with these functions: When doing these changes in my project originally, I converted the macros to C++ 11 lambdas so that I would have an easier time understanding the code and making the changes (here's how that looks). Once I decided to upstream the changes, I then converted the lambdas to functions to avoid the need for C++ 11. But the aspect of keeping the code local is unfortunately lost that way.

you can update the changelog at the top of the imgui_impl_sdl.cpp file.

Done, let me know if that looks ok!

can SDL_GameControllerOpen() return NULL and if so what will happen on SDL_GameControllerClose() ?

Ah yes, good catch, it can indeed return NULL. I've changed the code to only add it to the list if the return value is non-null. Please let me know if doing this type of combined assignment & check in an if-statement is ok, or if you'd prefer to have the assignment as a separate statement.

[ocornut]

My suspicion is that earlier versions of SDL (I'm using 2.0.14) did not require this lib, so the build worked fine, but some newer version now needs it. But I need to verify that. Let me know if you'd prefer a separate PR for that change, as it is indeed unrelated to the rest of this PR.

Yes I would like us to clarify that, I honestly think it would be surprising if SDL suddenly started requiring shell32 especially for such a a simple version it would usually prefer experimenting.

The spacing in the first two functions is also odd. We normally have this specced in the .editor_config file but this is not well handled by all IDE..

[lethal-guitar]

Yup, I just tested with an older version of SDL, and indeed the linker error does not happen with that one. So it seems my suspicion was correct, but I'll try to track down more precisely in which version that changed and why..

In the meantime, I've removed the commit and also fixed the spacing (I'm using Vim 😄)

[lethal-guitar]

Ok, I managed to track it down in the SDL source. The dependency on Shell32.lib was introduced in version 2.0.12, with this commit: https://github.com/libsdl-org/SDL/commit/f8400cbba9bb5cc64fdbe4b14729ff8497700a1e

[ocornut]

I have pushed bf1c96d+f966da1 which:

• add support for disconnection/reconnection
• auto mode picks first controller
• add ImGui_ImplSDL2_SelectGamepadExplicit() to select a gamepad manually.

I am not entirely against an API for multiple-gamepad support but I'm not entirely sure this is what you actually truly needed (vs simply better handling or disconnection/reconnection).

[ocornut]

I reopened as I am going to work those API right now. Support for multiple gamepads requires some rework as we now use io events API, so it needs to be done differently.

[ocornut]

Rework API and added support for multiple controllers: d15e410

IMGUI_IMPL_API void ImGui_ImplSDL2_SetGamepadModeAutoFirst();   // Use first available gamepad (default)
IMGUI_IMPL_API void ImGui_ImplSDL2_SetGamepadModeAutoAll();
IMGUI_IMPL_API void ImGui_ImplSDL2_SetGamepadModeManual(struct _SDL_GameController** gamepads_array, int gamepads_count);

[lethal-guitar]

@ocornut nice, that new auto-all mode looks exactly like what I need. Thanks!

[ocornut]

Reworked API it is now:

enum ImGui_ImplSDL2_GamepadMode 
{ 
   ImGui_ImplSDL2_GamepadMode_AutoFirst, 
   ImGui_ImplSDL2_GamepadMode_AutoAll, 
   ImGui_ImplSDL2_GamepadMode_Manual 
};
IMGUI_IMPL_API void ImGui_ImplSDL2_SetGamepadMode(ImGui_ImplSDL2_GamepadMode mode, struct _SDL_GameController** manual_gamepads_array = NULL, int manual_gamepads_count = -1);

SDL3 has same API. Used "Gamepad" in function names to match SDL3 code and make things easier to diff.