| Summary: | spurious SDL_MOUSEMOTION events with SDL_HINT_MOUSE_RELATIVE_MODE_WARP 1 since Windows 10 Fall Creators update | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Elisée Maurer <eliseegw+libsdl> |
| Component: | *don't know* | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | ewasylishen, metalcaedes, pickett.bill, sezeroz, szyk |
| Version: | HG 2.0 | ||
| Hardware: | x86 | ||
| OS: | Windows 10 | ||
| Attachments: |
Minimal program to reproduce the bug
(proposed fix for the issue; probably not ideal) patch with workaround that's only used on Win10 FCU and newer |
||
|
Description
Elisée Maurer
2017-10-31 23:41:30 UTC
We reproduced this. Looks like an OS bug. This affects SDL-1.2 too: SDL-1.2 builds of quakespasm suffers very badly from random mouse warps on win10 with fall update. And obviously, you don't need to set any hints or anything in order to trigger it. Note that this *could* happen even without the hint: SDL2 falls back to warping if raw mouse input (via RegisterRawInputDevices()) fails - apparently "security" software (like Norton Internet Security) sometimes does that, see https://bugzilla.libsdl.org/show_bug.cgi?id=2368 Does anyone have any workarounds for this then? It is annoying and makes the game pretty much unpplayable. I am experiencing this issue with Amnesia: The Dark Descent (GOG version). I _HAVE_ found a work-around. In this post: https://www.frictionalgames.com/forum/thread-7878-post-70551.html#pid70551 Download SDL 1.2.13 and in the Amnesia installation folder rename SDL.dll 1.2.14 to something else to keep a backup copy and then place SDL.dll 1.2.13 into the Amnesia folder. This, at least for me and at least for Amnesia: The Dark Descent, is a work-around. The game with the replacement SDL.dll file now works correctly. This is only a work-around though - if it's an OS bug then a real fix should come from Microsoft. Knowing that 1.2.13 works and 1.2.14 doesn't will also likely be helpful to bisect the bug. Sorry, should have mentioned: 1.2.15 _doesn't_ work for me, 1.2.13 does. The above fix as I've given it, SDL 1.2.13, also fixes various ScummVM games for myself. ScummVM comes with SDL 1.2.15. Bisect shows, that this conditional statement causes currently the mouse glitch on Windows 10: https://hg.libsdl.org/SDL/rev/a6f635e5eaa6#l5.24 . I hope, that somebody can prepare the fix for SDL 1.2.15 ... I mean https://hg.libsdl.org/SDL/rev/a6f635e5eaa6#l5.23 (and exactly the line: https://hg.libsdl.org/SDL/rev/a6f635e5eaa6#l5.25 ). That commit https://hg.libsdl.org/SDL/rev/a6f635e5eaa6#l5.24 is just adding the "warping" implementation, though (GetCursorPos / SetCursorPos based). Broken GetCursorPos / SetCursorPos based games on Win 10 fall creators are not limited to SDL.. I just tested winquake.exe (original 1997 exe) and it now has "jumps" in the mouse input if you try to look around in a circle. It uses GetCursorPos/SetCursorPos by default. Switching WinQuake to use directinput (-dinput flag) seems to get rid of the jumps. I've debugged the problem, my findings are at https://discourse.libsdl.org/t/win10-fall-creators-update-breaks-mouse-warping/23526/2?u=daniel_gibson I got from "why does this not work" to "why did this ever work", so I guess that's something.. at least I'm hopeful now that the bug can be fixed in SDL. Created attachment 3110 [details] (proposed fix for the issue; probably not ideal) This patch should fix the issue. The issue is, that (before this fix) "xrel = x - mouse->last_x;" is used to calculate the relative movement (same for yrel), and later "mouse->last_x = x;" is set, but the cursor is also reset to the center of the window. So next frame the correct calculation for xrel would be "x - center_x;" (like it is now), and "x - mouse->last_x" will return the wrong values: Imagine center_x being 240, last_x being 260 and x being 250, then with the old code xrel is -10, now it's 10, like it should. I think the only reason the old code kinda worked is, that (at least on Linux/X11) there are enough mouse movement events without any movements, which would reset mouse->last_x to center_x (same for ..y). More details at https://discourse.libsdl.org/t/win10-fall-creators-update-breaks-mouse-warping/23526/2 and following. (In reply to Daniel Gibson from comment #11) > Created attachment 3110 [details] > proposed fix for the issue > > This patch should fix the issue. Do you have a fix for the SDL-1.2 branch? The issue is really showing itself in the SDL-1.2 branch. no, haven't reallly looked at the 1.2 code. on first glance it looks pretty different and I currently don't feel like trying to understand it. TBH I wonder why they apparently switched from directinput for mouse input to warping in 1.2.14?! either way, projects should really upgrade to SDL2 (and does anyone know the state of that SDL1.2 for SDL2 wrapper that has been worked on at some point?) It's great that you worked out how to fix SDL2. 1.2 should be fixed but at least on Windows that's not the biggest problem. The biggest problem, again at least on Windows, is all the existing software that ships with SDL included. It isn't very likely that all the software is going to be updated and in the worst case if someone statically linked SDL into their executable then it may never be possible to fix it without developer support. The best solution, next to fixing SDL itself, is as you mentioned on Discourse that Microsoft should be made aware of the issue and fix the regression. Of course SDL still won't be perfect but at least it should be functional from the user-standpoint with that. > The biggest problem, again at least on Windows, is all the existing software > that ships with SDL included. It isn't very likely that all the software is > going to be updated I think that for software that's not updated anymore giving users the option to replace SDL.dll themselves is ok. People do more obscure things to get old games to work.. > and in the worst case if someone statically linked SDL into their executable > then it may never be possible to fix it without developer support. I don't think this was very popular with SDL1.2, as the license (LGPL) doesn't allow this (unless the program is open source, in which case creating new binaries should be possible) > The best solution, next to fixing SDL itself, is as you mentioned on Discourse > that Microsoft should be made aware of the issue and fix the regression. The problem is, that at least with SDL2 it was a clear bug in SDL and Windows didn't do anything wrong, as far as I can tell. Relying on getting enough mouse motion events without actual motion is just too fragile.. (I don't think this was intended, it probably was just an oversight and no one noticed because it worked) However, someone (not me, please) should debug the problem in SDL1.2 (and maybe winquake) to figure out if there is a real bug in Windows, maybe when using SetCursorPos() + GetCursorPos() (instead of SetCursorPos() and the normal mouse motion events) Damn, I just noticed that maybe those "mouse motion events without actual motion" aren't random after all, but are the result of SDL_WarpMouseInWindow(), which calls SetCursorPos(). Which would mean that with the Fall Creators Update, Win10 stopped sending a mouse motion event for SetCursorPos() - and brief testing with updated Win10 vs WinXP suggest that this is indeed the case. That sounds like a serious regression in Win10 after all. Comment on attachment 3110 [details] (proposed fix for the issue; probably not ideal) A friend tested on Win10 1607 (which is before the Fall Creators Update) and the the bug doesn't occur there, so the regression that SetCursorPos() doesn't reliably generate mouse events was indeed introduced with that update. I even reproduced it in a minimal WinAPI-only application (https://gist.github.com/DanielGibson/b5b033c67b9137f0280af9fc53352c68), the weird thing is that if you don't do anything each "frame" (i.e. the mainloop only polls the events and does nothing else), there are a lot of mouse events with the coordinates you passed to SetCursorPos(), but when sleeping for 10ms in each iteration of the mainloop, those events basically don't happen anymore. Which is bad, because in games the each iteration of the mainloop usually takes 16ms.. However, I think that the patch I suggested is not a great solution after all - it looks like it can indeed happen that you get several mouse events before the cursor is reset again, mouse->last_x is more correct. Problem is, I don't have a good idea for another solution either (except for Microsoft fixing the bug, but their bugtracker is readonly, so I have no idea how to report....) /Maybe/ the whole mouse warping implementation could be changed to (at least on Windows) ignore all system mouse events and only use SetCursorPos() and GetCursorPos() (the latter could be done when SDL_PumpEvents() is called) - but this feels like quite an intrusive change and I'm not even sure that warping with SetCursorPos() and GetCursorPos() works reliably after the update either, as at least winquake.exe (which seems to do this? haven't checked myself) is affected as well. Or we could check for the running(!) windows version and reset mouse->last_x/y after every mouse event only on affected systems? But that's quite hacky as well... OTOH, on the relevant platforms relative mouse mode via warping only is only a hack/fallback anyway, so maybe it's ok if it's a bit wonky, as long as it's not totally broken like it is right now on Win10? So maybe my proposed patch is good enough after all (would be nice to have a way to tell people that raw input doesn't work on their system and they should try to fix it though)? Created attachment 3112 [details] patch with workaround that's only used on Win10 FCU and newer I have a patch now that I find acceptable. It checks for the windows version with RtlGetVersion() (https://msdn.microsoft.com/en-us/library/windows/hardware/ff561910.aspx) and only if it's >= Win10 build 16299, enables the workaround. All code is in video/windows/SDL_windowsevents.c and the workaround is, that for each WM_MOUSEMOVE event, "if(isWin10FCUorNewer && mouseID != SDL_TOUCH_MOUSEID && mouse->relative_mode_warp)", an addition mouse move event is generated with the coordinates of the center of the screen (SDL_SendMouseMotion(data->window, mouseID, 0, center_x, center_y);) - which is exactly what would happen if windows generated those reliably itself. This will cause SDL_PrivateSendMouseMotion() to set mouse->last_x = center_x; and mouse->last_y = center_y; so the next mouse relative mouse event will be calculated correctly. If Microsoft ever fixes this bug, the IsWin10FCUorNewer() function would have to be adjusted to also check for a maximum version, so the workaround is then disabled again. In my VS2005 builds of SDL2, ntdll.dll is not linked in, same with my mingw-w64 builds: I am guessing that GetModuleHandle() works here if ntdll.dll is pulled in implicitly by kernel32.dll? Out of curiosity: does the good old GetVersionEx() not work for this same purpose?? > Out of curiosity: does the good old GetVersionEx() not work for this > same purpose?? OK, I take my question back; GetVersionEx() API change: https://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85).aspx Thanks guys, this is a crazy one! Daniel, your patch is in: https://hg.libsdl.org/SDL/rev/c70cf178aacb Thanks for merging, Sam! @Ozkan: I only tested with VS2013; can you test if it works with VS2005 or MinGW? (you could add a call to SDL_Log() if GetModuleHandleW() fails/succeeds) Otherwise GetModuleHandleW() should be replaced with LoadLibrary(). (I tested the SDL2.dll built with VS2013 and with a SDL_Log() printing the windows version from RtlGetVersion() in a WinXP VM and it seemed to work even there.. but I didn't test different compilers.) (In reply to Daniel Gibson from comment #22) > Thanks for merging, Sam! We really need an SDL-1.2 backport of this: The issue can't be easily hit with SDL2, but it _is_ hit with SDL1.2. > @Ozkan: I only tested with VS2013; can you test if it works with VS2005 or > MinGW? (you could add a call to SDL_Log() if GetModuleHandleW() > fails/succeeds) > Otherwise GetModuleHandleW() should be replaced with LoadLibrary(). Will try as soon as I'm able. I've reported the bug to Microsoft using their "Feedback Hub" Win10 App. If you're running Win10, the following link should open the issue in Feedback Hub where you could vote for it: https://aka.ms/W2icb9 Google Chrome ran into the same problem and reported it to Microsoft who will fix it "for the next major release", whenever that will happen: https://bugs.chromium.org/p/chromium/issues/detail?id=781182#c15 In that case we need the changing ">= 16299" to "== 16299" in the fixed merged for SDL2, yes? As we don't know what version this will be fixed in (and if Microsoft gets the fix right), I'm not sure about that Does anyone know whether this issue is fixed in the upcoming Windows 10 update? I checked, that in Windows 10 April 2018 Update the bug is fixed. (In reply to aurhat from comment #29) > I checked, that in Windows 10 April 2018 Update the bug is fixed. I submitted a patch to restrict the workaround to win10-v1709 in bug #4152 For anyone not following #4152: I don't think Microsoft fixed the bug, more details at https://bugzilla.libsdl.org/show_bug.cgi?id=4152#c8 |