SDL back-end: testing window id

[Helco]

The function signature for the processing of SDL events looks like this

bool ImGui_ImplSDL2_ProcessEvent(SDL_Event* event)

Is there a reason as to why event should be modifiable (the current implementation does not write into it)? If not it would be good to declare it as const SDL_Event* event. Also the current implementation does not check whether the event belongs to the SDL_Window used to initialize the implementation, which produces bugs in multi-window applications. Whether this is supposed to be filtered by the calling function or not, this should be documented somehow.

[ocornut]

You are right that it should be const, I will make that change. I guess when I added it I wasn't sure if there was a use for writing into the event structure, but I don't think there is a need.

SDL doesn't explicitly associate events to window, so there's no concept of "belonging" to a SDL_Window. It's up to the user to track the window events if they want to associate things like key/mouse events to a certain window (and how to interpret/react on mouse enter/leave, focus/unfocus is up to the user).

For the use we make in imgui_impl_sdl.cpp (both master and viewport branch, which is more evolved) we don't do any tracking because it doesn't matter. However:

which produces bugs in multi-window applications

I presume you are talking about multiple SDL window where only 1 window is used for an imgui context (NOT using imgui multiple-viewport) and the other window(s) don't have anything to do with imgui. To handle this case indeed we'd need perform some tracking of the window events and filtering accordingly.

This can also be done on the user's side and is probably more flexible on the user's side as you can decide which focusing rule you want to apply. If you attempt this in your app could you post your code/pseudo-code here for references, and to see if it would make sense replicating it in imgui_impl_sdl.cpp? Thank you.

[Helco]

In most of the input-related event structures of SDL there is a windowID member which can be compared with the the window ID retrievable by the SDL_GetWindowID function. Thus my current filtering before I pass the SDL_Event to imgui looks like this:

Uint32 getWindowIDByEvent(const SDL_Event* ev)
{
    switch (ev->type)
    {
        case (SDL_KEYDOWN):
        case (SDL_KEYUP):
            return ev->key.windowID;
        case (SDL_MOUSEBUTTONDOWN):
        case (SDL_MOUSEBUTTONUP):
            return ev->button.windowID;
        case (SDL_MOUSEMOTION):
            return ev->motion.windowID;
        case (SDL_MOUSEWHEEL):
            return ev->wheel.windowID;
        case (SDL_WINDOWEVENT):
            return ev->window.windowID;
        case (SDL_TEXTINPUT):
            return ev->text.windowID;
    }
    return UINT32_MAX;
}

if (getWindowIDByEvent(ev) == SDL_GetWindowID(window))
    ImGui_ImplSDL2_ProcessEvent(ev);

[ocornut]

Oh that's very good, I completely missed on this field. Thanks! (It is a shame they didn't put it in the base structure but I assume they needs to keep ABI compatibility.)

I'll look into the possibility of adding similar filtering code in imgui_impl_sdl.cpp that will support both master/viewport branches.

[Helco]

Great, I'm looking forward to that

[ocornut]

A PR would also be welcome, I don't know when I'll be able to look at this.