Split ImGuiImplSDL class into two for the SDL2 and SDL3 backends.

[triplejam]

The two pairs of files are almost identical, only the SDL number is different. I think this way makes it clear which *backend versions are used and supported.

[TheMostDiligent]

The two pairs of files are almost identical, only the SDL number is different

That seems like a problem TBH.
If everything is identical, then it may be better to add branching in the code. Like, depending on the version, call either SDL2 or SDL3.
Or initialize the jump table in the constructor depending on the version

[triplejam]

I thought about that. I wasn't sure if branching would be messier. I am not sure if there is a convenient way to automatically determine if version 2 or 3 is being used. I will rewrite this.

[TheMostDiligent]

I think version should be passed as parameter. Adding enum like to clearing indicate supported versions

enum SDL_BACKEND_VERSION
{
    SDL_BACKEND_VERSION_2 = 2,
    SDL_BACKEND_VERSION_3 = 3
 };

[triplejam]

There's a bit of an annoying problem if it's done this way. Since the user has to link to the implementation, the backend functions cannot be directly referenced on the implementation side because other version's will be missing. I'm guessing you may be alluding to a solution by "initialize the jump table" in the constructor. That would be on the interface side. But even then using the enum as a parameter would not work at runtime. So either it needs passed as template parameter or maybe more simply, forgo the enum and provide two inline factory functions like CreateVersion2/3() that would be passed instead to the constructor.

So something like

inline ImGuiImplSDL::VersionImpl ImGuiImplSDL::CreateSDL2VersionImpl()
{
    extern bool ImGui_ImplSDL2_InitForOpenGL(SDL_Window * window, void* sdl_gl_context);
    extern bool ImGui_ImplSDL2_InitForVulkan(SDL_Window * window);
    extern bool ImGui_ImplSDL2_InitForD3D(SDL_Window * window);
    extern bool ImGui_ImplSDL2_InitForMetal(SDL_Window * window);
    extern bool ImGui_ImplSDL2_InitForSDLRenderer(SDL_Window * window, SDL_Renderer * renderer);
    extern bool ImGui_ImplSDL2_InitForOther(SDL_Window * window);
    extern void ImGui_ImplSDL2_Shutdown();
    extern void ImGui_ImplSDL2_NewFrame();
    extern bool ImGui_ImplSDL2_ProcessEvent(const SDL_Event* event);
    // clang-format off
    return VersionImpl(ImGui_ImplSDL2_InitForOpenGL,
                       ImGui_ImplSDL2_InitForVulkan,
                       ImGui_ImplSDL2_InitForD3D,
                       ImGui_ImplSDL2_InitForMetal,
                       ImGui_ImplSDL2_Shutdown,
                       ImGui_ImplSDL2_NewFrame,
                       ImGui_ImplSDL2_ProcessEvent);
    // clang-format on
}

and that gets passed to the constructor:

static std::unique_ptr<ImGuiImplSDL>
Create(const ImGuiDiligentCreateInfo& CI, VersionImpl pVersionImpl, SDL_Window* pWindow);

Unless there is a better idea.

[TheMostDiligent]

Looks good - just a couple of minor comments

[triplejam]

Not sure if you meant removing the initial brace of the switch. If I remove it then It doesn't look right after running clang-format, so I left it in.

[TheMostDiligent]

I noticed a few minor naming issue + there are build errors on Appveyor:

  ImGuiImplSDL3.cpp
C:\projects\DiligentTools\Imgui\src\ImGuiImplSDL2.cpp(107,69): error C2440: 'initializing': cannot convert from 'Diligent::ImGuiImplSDL2::GAMEPAD_MODE' to 'ImGui_ImplSDL2_GamepadMode' [c:\projects\build\DiligentTools\Imgui\Diligent-Imgui.vcxproj]
C:\projects\DiligentTools\Imgui\src\ImGuiImplSDL2.cpp(107,44): message : Conversion to enumeration type requires an explicit cast (static_cast, C-style cast or function-style cast) [c:\projects\build\DiligentTools\Imgui\Diligent-Imgui.vcxproj]
  ImGuiImplWin32.cpp
C:\projects\DiligentTools\Imgui\src\ImGuiImplSDL3.cpp(97,69): error C2440: 'initializing': cannot convert from 'Diligent::ImGuiImplSDL3::GAMEPAD_MODE' to 'ImGui_ImplSDL3_GamepadMode' [c:\projects\build\DiligentTools\Imgui\Diligent-Imgui.vcxproj]
C:\projects\DiligentTools\Imgui\src\ImGuiImplSDL3.cpp(97,44): message : Conversion to enumeration type requires an explicit cast (static_cast, C-style cast or function-style cast) [c:\projects\build\DiligentTools\Imgui\Diligent-Imgui.vcxproj]