Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backends: SDL2, SDL3: ignore events of other windows #7853

Closed
wants to merge 1 commit into from

Conversation

madebr
Copy link
Contributor

@madebr madebr commented Aug 1, 2024

When creating multiple SDL2/SDL3 windows, of which not all windows use imgui,
it is expected that imgui ignores all events of other windows.

This pull requests checks the windowID of the event and only acknowledges the event if it targets the current window.

Patch of SDL3 example that creates a second SDL windows

Mouse movements and/or clicks in the 2nd window should be ignored by imgui.

diff --git a/examples/example_sdl3_sdlrenderer3/main.cpp b/examples/example_sdl3_sdlrenderer3/main.cpp
index 6efd2754..327aaaaa 100644
--- a/examples/example_sdl3_sdlrenderer3/main.cpp
+++ b/examples/example_sdl3_sdlrenderer3/main.cpp
@@ -49,6 +49,17 @@ int main(int, char**)
     SDL_SetWindowPosition(window, SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED);
     SDL_ShowWindow(window);
 
+
+    SDL_Window* window2 = SDL_CreateWindow("2nd window", 640, 480, SDL_WINDOW_RESIZABLE);
+    if (window == nullptr)
+    {
+        printf("Error: SDL_CreateWindow(): %s\n", SDL_GetError());
+        return -1;
+    }
+    SDL_Color render2_color = { 0 };
+    SDL_Renderer* renderer2 = SDL_CreateRenderer(window2, nullptr);
+
+
     // Setup Dear ImGui context
     IMGUI_CHECKVERSION();
     ImGui::CreateContext();
@@ -110,7 +121,36 @@ int main(int, char**)
                 done = true;
             if (event.type == SDL_EVENT_WINDOW_CLOSE_REQUESTED && event.window.windowID == SDL_GetWindowID(window))
                 done = true;
+            switch (event.type) {
+            case SDL_EVENT_WINDOW_CLOSE_REQUESTED:
+                if (event.window.windowID == SDL_GetWindowID(window2)) {
+                    SDL_DestroyRenderer(renderer2);
+                    renderer2 = nullptr;
+                    SDL_DestroyWindow(window2);
+                    window2 = nullptr;
+                }
+            case SDL_EVENT_MOUSE_MOTION:
+                if (event.motion.windowID == SDL_GetWindowID(window2)) {
+                    int w, h;
+                    SDL_GetWindowSizeInPixels(window2, &w, &h);
+                    render2_color.r = (int)(255.5f * (event.motion.x / w));
+                    render2_color.g = (int)(255.5f * (event.motion.y / h));
+                }
+                break;
+            case SDL_EVENT_MOUSE_BUTTON_UP:
+                if (event.button.windowID == SDL_GetWindowID(window2)) {
+                    render2_color.b = (render2_color.b + 0x3f) % 256;
+                }
+                break;
+            }
+        }
+
+        if (renderer2) {
+            SDL_SetRenderDrawColor(renderer2, render2_color.r, render2_color.g, render2_color.b, render2_color.a);
+            SDL_RenderClear(renderer2);
+            SDL_RenderPresent(renderer2);
         }
+
         if (SDL_GetWindowFlags(window) & SDL_WINDOW_MINIMIZED)
         {
             SDL_Delay(10);
@@ -176,6 +216,9 @@ int main(int, char**)
     ImGui_ImplSDL3_Shutdown();
     ImGui::DestroyContext();
 
+    SDL_DestroyRenderer(renderer2);
+    SDL_DestroyWindow(window2);
+
     SDL_DestroyRenderer(renderer);
     SDL_DestroyWindow(window);
     SDL_Quit();

@ocornut
Copy link
Owner

ocornut commented Aug 2, 2024

Hello,
Thanks for the PR.
Your code is responsible for calling ImGui_ImplSDL2_ProcessEvent(), so technically you could do the filtering on your end if you are in a position where you have other windows created, which may be considered a little bit of a fringe use case.

The function says:

// If you have multiple SDL events and some of them are not meant to be used by dear imgui, you may need to filter events based on their windowID field.

But I agree it'll be simpler if the backend does it because windowID is part of each individual events and in theory which events are being used by the backend is opaque.

I'll look into reworking this to support multi-viewports too.

@madebr
Copy link
Contributor Author

madebr commented Aug 2, 2024

I asked on SDL's discord about a function to easily extract the windowID from a SDL_Event, but there is currently none.
If you think such a function might be useful, please create an issue at https://github.com/libsdl-org/SDL/issues

@madebr
Copy link
Contributor Author

madebr commented Aug 3, 2024

SDL3 now has a function SDL_WindowFromEvent to easily get a SDL_Window associated with an event.
So instead of modifying imgui's backend code, you can filter the SDL event by doing:

extern SDL_Window *imgui_sdl_window;
SDL_Event event; /* After SDL_PollEvent(&event); */
SDL_Window *event_window = SDL_GetWindowFromEvent(&event);
if (event_window == imgui_sdl_window || event_window == NULL) {
    ImGui_ImplSDL3_ProcessEvent(&event);
}

@ocornut
Copy link
Owner

ocornut commented Aug 7, 2024

I think we could add the equivalent ImGui_ImplSDL2_GetWindowFromEvent() helper in our codebase too?

@ocornut
Copy link
Owner

ocornut commented Aug 19, 2024

Thank you for your work adding this to SDL3.

I realized that this function may unfortunately not be what we want, as some events are not associated to a window and yet useful for the backend, e.g. SDL_EVENT_DISPLAY_ADDED, SDL_EVENT_GAMEPAD_ADDED.

Unfortunately this means maybe even the newly added SDL_GetWindowFromEvent() may be "dangerous" in the sense that it would lead users to misleading uses. Sorry for that.

So either we provide a function with a different signature implying something else, e.g. bool ImGui_ImplSDL3_IsEventWantedByWindow() either we may need to do something that's basically your original PR. I think I will look into merging it.

@madebr
Copy link
Contributor Author

madebr commented Aug 19, 2024

Thank you for your work adding this to SDL3.

I realized that this function may unfortunately not be what we want, as some events are not associated to a window and yet useful for the backend, e.g. SDL_EVENT_DISPLAY_ADDED, SDL_EVENT_GAMEPAD_ADDED.

The behavior of the SDL function is to return a SDL_Window pointer if the event has a windowID member, and return NULL otherwise.
That's why the return value of SDL_GetWindowFromEvent is tested against the window and nullptr.

You're probably right the SDL3 function should be wrapped by a imgui helper function.
The additional nullptr check is easily forgotten.

e.g.

bool ImGui_ImplSDL3_IsEventWantedByWindow(const SDL_Event *event, SDL_Window *window) {
    SDL_Window *event_window = SDL_GetWindowFromEvent(event);
    return event_window == window || event_window == nullptr;
}

ocornut added a commit that referenced this pull request Aug 19, 2024
…apping into a function as it'll be convenient for multi-viewport check. (#7853)

+ Misc typo fix.
ocornut added a commit that referenced this pull request Aug 19, 2024
…formHandle instead of SDL_Window*. (#7853)

This will be used to support filtering of events with multi-viewports.
ocornut added a commit that referenced this pull request Aug 19, 2024
@ocornut
Copy link
Owner

ocornut commented Aug 19, 2024

Right, that would also work for single viewport mode.

Anyhow it's harder to figure out for multi-viewports so I believe it should be done in the backend as you initially suggested.

I have pushed for following:

  • 591a18a your change, at this point but that docking/multi-viewport would be broken.
  • 1b61d55 wrapped in a function also suitable for multi-viewports + storing WindowID for convenience.
  • 2d99052 change ImGuIViewport::PlatformHandle storage from SDL_Window* to Uint32/SDL_WindowID at it is more convenient.
  • ea01c63 (docking): amends to get things working for multi-viewports.

Thanks for your help!

@ocornut ocornut closed this Aug 19, 2024
@madebr madebr deleted the ignore-events-of-other-windows branch August 19, 2024 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants