Skip to content

Conversation

@triplejam
Copy link

@triplejam triplejam commented Nov 15, 2025

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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.

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.

2 participants