Poor SDL 1.2 win32 mouse input

[SDLBugzilla]

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• Patch that fixes poor win32 mouse input (sdl-win32-mouse-fixes.diff, text/plain, 2008-08-12 11:20:34 +0000, 8134 bytes)
• Version 2 of above (sdl-win32-mouse.diff, text/plain, 2008-08-30 12:15:09 +0000, 16660 bytes)
• Version 2.1 of above (sdl-win32-mouse.diff, text/plain, 2008-08-31 03:03:52 +0000, 16596 bytes)
• Version 2.2 of above (sdl-win32-mouse.diff, text/plain, 2008-09-01 14:18:49 +0000, 16914 bytes)
• A diff on the diff yo (numpadfix.patch, text/plain, 2009-03-31 07:45:49 +0000, 7166 bytes)
• Parts missing from subversion commit of above (sdl-wincommon-sdl_sysevents-chang, text/plain, 2009-04-02 08:35:09 +0000, 3047 bytes)
• s/_WIN32_WCE/SDL_VIDEO_DRIVER_GAPI/ (sdl-sdl_dibevents-gapi-correction, text/plain, 2009-04-13 02:58:11 +0000, 509 bytes)

Reported in version: HG 1.2
Reported for operating system, platform: Windows (All), x86

Comments on the original bug report:

On 2008-08-12 11:18:06 +0000, Tim Angus wrote:

[Copied from mailing list post I made on 11/08/08. Apologies for the fact this is somewhat of meta-bug, I hate getting these myself. At least in this case each of the individual bugs here are pretty tightly related.]

I'm one of the maintainers of ioquake3.org, an updated version of the

Quake 3 engine. Relatively recently, we moved ioq3 to use SDL as a

replacement for 95% of the platform specific code that was there. On the

whole it's doing a great job but unfortunately since the move we've been

getting complaints about the quality of the mouse input on the Windows

platform to the point where for many the game is unplayable. Put in

other terms, the current stable SDL 1.2 is basically not fit for purpose

if you need high quality mouse input as you do in a first person shooter.

Over the weekend I decided to pull my finger out and actually figure out

what's going on. There are basically two major problems. Firstly, when

using the "windib" driver, mouse input is gathered via the WM_MOUSEMOVE

message. Googling for this indicates that often this is known to result

in "spurious" and/or "missing" mouse movement events; this is the

primary cause of the poor mouse input. The second problem is that the

"directx" driver does not work at all in combination with OpenGL meaning

that you can't use DirectInput if your application also uses OpenGL. In

other words you're locked into using the "windib" driver and its poor

mouse input.

In order to address these problems I've done the following:

• Remove WM_MOUSEMOVE based motion event generation and replace with

calls to GetCursorPos which seems much more reliable. In order to

achieve this I've moved mouse motion out into a separate function that

is called once per DIB_PumpEvents.

• Remove the restriction on the "directx" driver being inoperable in

combination with OpenGL. There is a bug for this issues that I've

hijacked to a certain extent

(http://bugzilla.libsdl.org/show_bug.cgi?id=265). I'm the first to admit

I don't really understand why this restriction is there in the first

place. The commit message for the bug fix that introduced this

restriction (r581) isn't very elaborate and I couldn't see any other bug

tracking the issue. If anyone has more information on the bug that was

avoided by r581 it would be helpful as I/someone could then look into

addressing the problem without disabling the "directx" driver.

• I've also removed the restriction on not being allowed to use

DirectInput in windowed mode. I couldn't see any reason for this, at

least not from our perspective. I have my suspicions that it'll be

something like matching up the cursor with the mouse coordinates...

• I bumped up the DirectInput API used to version 7 in order to get

access to mouse buttons 4-7. I've had to inject a little bit of the DX7

headers into SDL there as the MinGW ones aren't up to date in this respect.

On 2008-08-12 11:20:34 +0000, Tim Angus wrote:

Created attachment 263
Patch that fixes poor win32 mouse input

Bah, copying and pasting that didn't work out so well. Anyway, here is the patch.

On 2008-08-12 11:22:33 +0000, Tim Angus wrote:

*** Bug 265 has been marked as a duplicate of this bug. ***

On 2008-08-30 12:15:09 +0000, Tim Angus wrote:

Created attachment 267
Version 2 of above

This new patch, I believe, addresses the issues that prevented DirectInput being used in windowed/OpenGL mode originally. DI is a relative input system and thus has no notion of absolute positioning and mouse cursors. Since SDL (currently) assumes no control over the mouse cursor, DI won't operate like the other drivers in absolute mode. This update fixes this issue by assuming control of the cursor when it's within the SDL window.

Also fixed is a problem where SDL_APPMOUSESTATE notifications ceased to be updated following a second call to SDL_SetVideoMode, due to the static variable in_window being out of sync.

On 2008-08-31 03:03:52 +0000, Tim Angus wrote:

Created attachment 269
Version 2.1 of above

Fix a stupid bug relating to focus with the windib driver.

On 2008-08-31 20:40:39 +0000, Gregory Smith wrote:

I tried applying this patch and unfortunately, I lose all mouse control in full screen directx mode, including cursor. It used to work with directx as long as OpenGL wasn't enabled, but with this patch, it doesn't work at all fullscreen.

Seems to work in windowed mode directx and windib.

Built with mingw 3.4

On 2008-09-01 01:55:20 +0000, Tim Angus wrote:

Can you provide a test case?

On 2008-09-01 04:59:23 +0000, Gregory Smith wrote:

Sure thing; testcursor.exe works normally, but if I modify it to start up full screen:

155c155
< screen = SDL_SetVideoMode(320,200,8,SDL_ANYFORMAT);

screen = SDL_SetVideoMode(640,480,8,SDL_FULLSCREEN);

The cursor stops moving. Mouse clicks do work.

On 2008-09-01 14:18:49 +0000, Tim Angus wrote:

Created attachment 271
Version 2.2 of above

OK, there were a couple of things going on there. Firstly, when fullscreen DI was being granted exclusive access to the mouse. This meant that WM_MOUSEMOVE messages didn't get delivered and SDL_APPMOUSEFOCUS never got set. Incidentally, I've changed how this works a bit and it was probably only by "chance" that this worked before. Secondly, as a consequence of exclusive access being granted, the mouse cursor was hidden and the software cursor was being used. As the WM cursor is now forced to track the absolute state of the mouse position, it can be used instead of the software cursor.

In hindsight this is probably the reason that DI was disabled with OpenGL; you can't draw a software cursor on a GL surface.

Anyway, it seems to all work with testcursor.c and my other tests now... fingers crossed this is the last patch :).

On 2008-09-02 19:37:39 +0000, Gregory Smith wrote:

Version 2.2 is definitely getting closer! But now I have problems launching with the windib driver, which I tracked down to SDL_SetGammaRamp. Sure enough, testgamma.exe fails with version 2.2 in windib :(

Why would changing the mouse code affect this, though?

On 2008-09-03 02:40:00 +0000, Tim Angus wrote:

(In reply to comment # 9)

Version 2.2 is definitely getting closer! But now I have problems launching
with the windib driver, which I tracked down to SDL_SetGammaRamp. Sure enough,
testgamma.exe fails with version 2.2 in windib :(

Why would changing the mouse code affect this, though?

The simple answer is that it shouldn't :). The only thing in there that could really be connected is that it checks if the app has SDL_APPINPUTFOCUS before setting the gamma. Even if the conditions here had changed though, I don't see how this would cause anything to fail. Speaking of which how does it fail? It just doesn't start at all?

In any case, I'll take a look tonight.

On 2008-09-03 07:04:34 +0000, Gregory Smith wrote:

(In reply to comment # 10)

Speaking of which how does it fail? It just doesn't start at all?

Segfaults in SDL_SetGammaRamp, IIRC.

On 2008-09-03 13:24:47 +0000, Tim Angus wrote:

Hmm, it works fine for me. Though interestingly the official 1.2.13 SDL.dll crashes where you say it does. It is possible it was compiled with the same compiler that you're using and the compiler has a bug? Here is my SDL.dll, with the patches attached to bugs # 611, # 618 and # 619 applied:

http://icculus.org/~tma/SDL.dll

tma@abraxas:~$ i586-mingw32msvc-gcc --version
i586-mingw32msvc-gcc (GCC) 4.2.1-sjlj (mingw32-2)

Let us know if this works for you?

On 2008-09-03 13:47:19 +0000, Gregory Smith wrote:

I will give your DLL a try (I will have to recompile because I usually use a static .lib), but stock SDL-1.2.13 and previous versions of your patch do not exhibit this problem for me. So, I am dubious that my (old) compiler is at fault.

On 2008-09-03 14:53:27 +0000, Tim Angus wrote:

Please try http://libsdl.org/release/SDL-1.2.13-win32.zip as well if that's not what you tried before. (I assume it isn't because there is no static lib in this release).

On 2008-09-03 16:59:30 +0000, Gregory Smith wrote:

Sure enough, your DLL works. The SDL-1.2.13 DLL does not. Static builds are just the opposite.

I am truly confused.

On 2008-09-04 01:51:30 +0000, Tim Angus wrote:

Perhaps give a different compiler a shot and see if it changes the behaviour you see? In any case I think (I hope!) we can chalk this one up as unrelated to the bug on topic here :).

On 2008-09-04 04:35:27 +0000, Gregory Smith wrote:

Yes, I will try to get 4.2.1 installed--it's very difficult--and give it a try. Until then, seems valid to assume this patch is not at fault.

On 2008-09-04 18:00:06 +0000, Gregory Smith wrote:

It was not my compiler, it was this: http://bugzilla.libsdl.org/show_bug.cgi?id=622

After fixing that, I was able to verify that mouse control works in all the situations the way I expect using your patch. Works great, thanks a lot for this.

On 2009-03-31 07:45:49 +0000, Sam Lantinga wrote:

Created attachment 311
A diff on the diff yo

On 2009-04-01 21:44:37 +0000, Sam Lantinga wrote:

I added version 2.2 of your patch to subversion. I wasn't sure whether or not the "diff on the diff" was necessary, as it didn't apply cleanly. Can you confirm whether it is?

On 2009-04-02 08:35:09 +0000, Tim Angus wrote:

Created attachment 316
Parts missing from subversion commit of above

On 2009-04-02 08:45:52 +0000, Tim Angus wrote:

First of all, thanks for committing this. Unfortunately it seems the patch has only partially applied to wincommon/SDL_sysevents.c and currently a clean SDL 1.2 checkout doesn't build. The new patch here (http://bugzilla.libsdl.org/attachment.cgi?id=316) fixes this.

The infamous diff of the diff (yo) isn't actually pertinent to this bug. Allow me to explain: we have been distributing a patch to SDL along with ioq3 that included the patches attached to SDL bugs 611, 618 and 619. A third party improved on this patch, but submitted it as a patch to this patch. I have now separated this and added it to bug 618.

In summary, if you could apply the following patches from 611, 618 and 619 respectively, it would mean we no longer need to distribute the patch (or use a patched SDL.dll when a new 1.2 release is made):

http://bugzilla.libsdl.org/attachment.cgi?id=316
http://bugzilla.libsdl.org/attachment.cgi?id=314
http://bugzilla.libsdl.org/attachment.cgi?id=315

Cheers.

On 2009-04-12 17:10:28 +0000, Sam Lantinga wrote:

I'll take a look, thanks for the well organized patches and bug reports!

On 2009-04-13 01:35:57 +0000, Sam Lantinga wrote:

It looks like your diff removes some of the GAPI fixes. Was this intentional?

On 2009-04-13 01:38:31 +0000, Sam Lantinga wrote:

Ah, I see the comment that addressed that.

Patch committed, thanks!

On 2009-04-13 02:57:11 +0000, Tim Angus wrote:

(In reply to comment # 23)

I'll take a look, thanks for the well organized patches and bug reports!

No problem, I take the attitude that I should submit patches in a manner I'd like to have them submitted to me for my own stuff.

(In reply to comment # 24)

It looks like your diff removes some of the GAPI fixes. Was this intentional?

Now you mention it I have noticed one small issue. The guard SDL_VIDEO_DRIVER_GAPI has been introduced at some point between me originally writing this patch and now, and there is a call in sdl_dibevents.c which is guarded with the original _WIN32_WCE as the patch wasn't updated when this happened. Patch forthcoming, sorry I didn't catch this before.

On 2009-04-13 02:58:11 +0000, Tim Angus wrote:

Created attachment 318
s/_WIN32_WCE/SDL_VIDEO_DRIVER_GAPI/

Last one, I promise!

On 2009-05-07 05:46:03 +0000, Tim Angus wrote:

Looks like this was addressed in r4508. Thanks, closing.