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

spurious SDL_MOUSEMOTION events with SDL_HINT_MOUSE_RELATIVE_MODE_WARP 1 since Windows 10 Fall Creators update #2695

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 2 comments

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 11, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: Windows 10, x86

Comments on the original bug report:

On 2017-10-31 23:41:30 +0000, Elisée Maurer wrote:

Created attachment 3051
Minimal program to reproduce the bug

Hi,

The attached minimal program sets the SDL_HINT_MOUSE_RELATIVE_MODE_WARP to 1, enables relative mouse mode then logs all SDL_MOUSEMOTION xrel values as they happen.

When moving the mouse exclusively to the right:

  • On a Windows 10 installation before Fall Creators update (for instance, Version 10.0.15063 Build 15063), only positive values are reported, as expected
  • On a Windows 10 installation after Fall Creators update (for instance, Version 10.0.16299 Update 16299), a mix of positive and negative values are reported.

3 different people have reproduced this bug and have confirmed it started to happen after the Fall Creators update was installed. It happens with SDL 2.0.7 as well as latest default branch as of today.

It seems like some obscure (maybe unintended) Windows behavior change? Haven't been able to pin it down more yet.

(To force-upgrade a Windows installation to the Fall Creators update, you can use the update assistant at https://www.microsoft.com/en-us/software-download/windows10)

On 2017-11-06 05:32:26 +0000, Ozkan Sezer wrote:

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.

On 2017-11-21 13:26:09 +0000, Daniel Gibson wrote:

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

On 2017-11-22 06:23:17 +0000, Ozkan Sezer wrote:

Does anyone have any workarounds for this then? It is
annoying and makes the game pretty much unpplayable.

On 2017-11-25 01:54:45 +0000, Bill Pickett wrote:

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.

On 2017-11-25 01:57:44 +0000, Bill Pickett wrote:

Sorry, should have mentioned: 1.2.15 doesn't work for me, 1.2.13 does.

On 2017-11-25 03:55:14 +0000, Bill Pickett wrote:

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.

On 2017-12-02 21:48:34 +0000, aurhat wrote:

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 ...

On 2017-12-02 21:51:58 +0000, aurhat wrote:

I mean https://hg.libsdl.org/SDL/rev/a6f635e5eaa6#l5.23 (and exactly the line: https://hg.libsdl.org/SDL/rev/a6f635e5eaa6#l5.25 ).

On 2017-12-03 19:41:59 +0000, Eric Wasylishen wrote:

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.

On 2017-12-05 19:43:28 +0000, Daniel Gibson wrote:

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.

On 2017-12-06 16:56:03 +0000, Daniel Gibson wrote:

Created attachment 3110
(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.

On 2017-12-06 17:30:25 +0000, Ozkan Sezer wrote:

(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.

On 2017-12-06 17:51:22 +0000, Daniel Gibson wrote:

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?)

On 2017-12-06 18:06:37 +0000, Bill Pickett wrote:

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.

On 2017-12-06 18:19:19 +0000, Daniel Gibson wrote:

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)

On 2017-12-07 14:19:36 +0000, Daniel Gibson wrote:

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.

On 2017-12-08 17:13:54 +0000, Daniel Gibson wrote:

Comment on attachment 3110
(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)?

On 2017-12-10 04:18:05 +0000, Daniel Gibson wrote:

Created attachment 3112
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.

On 2017-12-10 06:12:15 +0000, Ozkan Sezer wrote:

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??

On 2017-12-10 06:25:02 +0000, Ozkan Sezer wrote:

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

On 2017-12-10 17:18:04 +0000, Sam Lantinga wrote:

Thanks guys, this is a crazy one!

Daniel, your patch is in:
https://hg.libsdl.org/SDL/rev/c70cf178aacb

On 2017-12-10 18:07:15 +0000, Daniel Gibson wrote:

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.)

On 2017-12-10 19:41:14 +0000, Ozkan Sezer wrote:

(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.

On 2017-12-11 20:38:27 +0000, Daniel Gibson wrote:

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

On 2017-12-19 00:34:38 +0000, Daniel Gibson wrote:

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

On 2017-12-20 06:52:57 +0000, Ozkan Sezer wrote:

In that case we need the changing ">= 16299" to "== 16299" in the
fixed merged for SDL2, yes?

On 2017-12-20 11:43:27 +0000, Daniel Gibson wrote:

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

On 2018-04-25 07:57:32 +0000, Ozkan Sezer wrote:

Does anyone know whether this issue is fixed in the upcoming Windows 10 update?

On 2018-05-01 17:46:46 +0000, aurhat wrote:

I checked, that in Windows 10 April 2018 Update the bug is fixed.

On 2018-05-01 20:36:11 +0000, Ozkan Sezer wrote:

(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

On 2018-05-07 01:34:15 +0000, Daniel Gibson wrote:

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

@Spatzendame
Copy link

Spatzendame commented Feb 6, 2022

So is the muse bug still not fixed in SDL1.2 source code?
I know Microsoft may has added a workaround in later Windows updates and Amnesia also got a new update to fix the mouse, but did SDL source code itself also got a fix? Back in the days downgrade SDL to version 1.2.13.0 got the mouse working again. But it would be nice to have a fix in the latest source code.

@sezero
Copy link
Contributor

sezero commented Feb 6, 2022

So is the muse bug still not fixed in SDL1.2 source code?

No.

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

No branches or pull requests

3 participants