SDL2 handles mouse events incorrectly with x11vnc

chrismile commented 8 months ago

Version/Branch of Dear ImGui:

Branch: master (most current version)

Back-ends:

imgui_impl_sdl2.cpp

Compiler, OS:

Ubuntu 22.04 + GCC

Full config/build information:

No response

Details:

My Issue/Question:

The SDL2 backend seems to incorrectly handle mouse events when streaming the screen content via x11vnc (https://github.com/LibVNC/x11vnc).

How to reproduce:

Stream the screen content of an Ubuntu 22.04 PC via x11vnc.
Open any of the SDL2 ImGui demos.
Move the mouse. Whenever the mouse passes a widget, it is not hightlighted. Whenever the user does a mouse click, the position is updated, however, the click event is not registered at the current mouse position, but the position the mouse was at when the last mouse click before that happened.

I can confirm that this neither happens with the GLFW backend, nor do other applications and games using SDL2 (like the game 0 A.D.) suffer from this problem.

What the weirdest thing about this is: When I add a std::cout whenever SDL_MOUSEMOTION is registered by the ImGui backend, an output is written to the terminal at every mouse move. I do not understand why mouse clicks after that do not use this updated position.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

No response

ocornut commented 8 months ago

Hello, You can use Metrics->Debug Log->IO to visualize events with more clarify and try to find out where this is coming from.

chrismile commented 8 months ago

Thanks for the hint. The debug log always shows "Processed: MousePos (new_x, new_y) (Mouse)", followed immediately by "Processed: MousePos (old_x, old_y) (Mouse)" whenever the mouse is moved. "new" is the current mouse position, "old" the position when a mouse button was clicked the last time. I.e., the new position is immediately always overwritten by the old one.

Whenever a click is performed, after processing the click, it first says "Remaining: MousePos (new_x, new_y) (Mouse)", and then "Processed: MousePos (new_x, new_y) (Mouse)".

chrismile commented 8 months ago

It seems like the problem is SDL_GetGlobalMouseState, which always returns the old value when using x11vnc.

Would you mind shortly explaining why this code in case of "MouseCanUseGlobalState" in "ImGui_ImplSDL2_UpdateMouseData" is even necessary? The comment says:

Fallback to provide mouse position when focused (SDL_MOUSEMOTION already provides this when hovered or captured)

Are there any situations where one would need ImGui to know the mouse position when the application is neither hovered nor captured?

ocornut commented 8 months ago

This was added in 98ce0132, when transitioning to using SDL_MOUSEMOTION. With SDL and without that code, it means we wouldn't get updated mouse coordinates when moving the mouse outside of client area (unless clicked/captured). While this is not 100% mandatory it feels consistent to provide this info.

Imagine if you have contents always following the mouse:

ImGui::SetNextWindowPos(ImGui::GetMousePos());
ImGui::SetTooltip("Always a tooltip");

We've been regularly having issues with SDL_GetGlobalMouseState() but I think it would be valuable if you looked into your copy of SDL see what is happening with this value? Also try to update SDL just in case.

which always returns the old value when using x11vnc.

What do you mean by "old value" ?

We could consider using an heuristic to always prioritize the SDL_MOUSEMOTION data and only use theSDL_GetGlobalMouseState() value if no SDL_MOUSEMOTION event happened since last update, but we'd only do that if we have a confirmation there's no other solution.

chrismile commented 8 months ago

What do you mean by "old value" ?

With that I mean: SDL_GetGlobalMouseState, when using x11vnc for accessing the desktop, returns the position where the last mouse click occurred. Not the position the mouse currently is actually at (as signaled through SDL_MOUSEMOTION).

We've been regularly having issues with SDL_GetGlobalMouseState() but I think it would be valuable if you looked into your copy of SDL see what is happening with this value?

I agree. I looked into SDL, and this is the code responsible for SDL_GetGlobalMouseState on X11: https://github.com/libsdl-org/SDL/blob/5b3ee51c6ca94f803a4ca77ce0dce08ee0426f80/src/video/x11/SDL_x11mouse.c#L419 I confirmed that the current code is almost the same as SDL 2.0.20 (the version shipped by Ubuntu 22.04; it only has an ifdef check for XInput2 instead of a runtime check).

I can't say if XInput2 is enabled in the version of SDL2 shipped with Ubuntu, but if it is, then I believe this behavior could be explained this way:

The position is only updated if global_mouse_changed is true.
It is set to true in SDL_x11xinput2.c if XI_RawMotion or XI_RawButtonRelease events are encountered by XInput2.
The XI_RawMotion event is not triggered by x11vnc.

This is also supported by this source: https://stackoverflow.com/questions/62448181/how-do-i-monitor-mouse-movement-events-in-all-windows-not-just-one-on-x11

Set mask to receive XI_RawMotion events. Because it's raw, XWarpPointer() events are not included, you can use XI_Motion instead.

If x11vnc uses XWarpPointer (which is highly likely, as the mouse position is transferred via the network), XI_RawMotion will never trigger, and as SDL doesn't listen to XI_Motion events, it will think the mouse position never changed. Only when a XI_RawButtonRelease event arrives, it will finally update the mouse position.

Thus, this is probably neither a bug in x11vnc (calling XWarpPointer for moving the mouse is perfectly OK) nor a bug in ImGui, but a bug in SDL2. While I can (and will) of course report this to the SDL project, it might be nice to have some form of circumvention of this SDL bug in the ImGui backend code.

We could consider using an heuristic to always prioritize the SDL_MOUSEMOTION data and only use the SDL_GetGlobalMouseState() value if no SDL_MOUSEMOTION event happened since last update, but we'd only do that if we have a confirmation there's no other solution.

Yes, this should at least fix the problem when the window is hovered or captured. I think the most optimal solution would be to dynamically load XQueryPointer/XGetWindowAttributes at runtime when the X11 SDL backend is used, and rather use them instead of the function provided by SDL. But as this problem only affects VNC servers and other software using XWarpPointer, I understand if you don't want to invest that much time into solving this problem.

chrismile commented 8 months ago

I have now created the issue report in the upstream repo: https://github.com/libsdl-org/SDL/issues/8827

chrismile commented 7 months ago

The SDL issue report has been added to the version 2.30.0 milestones. If you do not want to put additional time into circumventing this bug (which I would understand, as it is not an ImGui bug and will hopefully be fixed in the next SDL releases), it might be a simple solution to only add the SDL backend "x11" to global_mouse_whitelist in case SDL_VERSION_ATLEAST(2, 30, 0) holds.

icculus commented 7 months ago

The SDL bug is resolved in SDL's repository now, and the fix will be in the 2.30.0 release (which we are locking down soon).

ocornut / imgui