We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 611 - Poor SDL 1.2 win32 mouse input
Summary: Poor SDL 1.2 win32 mouse input
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: events (show other bugs)
Version: HG 1.2
Hardware: x86 Windows (All)
: P2 major
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords:
: 265 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-08-12 11:18 UTC by Tim Angus
Modified: 2009-05-07 05:46 UTC (History)
2 users (show)

See Also:


Attachments
Patch that fixes poor win32 mouse input (7.94 KB, patch)
2008-08-12 11:20 UTC, Tim Angus
Details | Diff
Version 2 of above (16.27 KB, patch)
2008-08-30 12:15 UTC, Tim Angus
Details | Diff
Version 2.1 of above (16.21 KB, patch)
2008-08-31 03:03 UTC, Tim Angus
Details | Diff
Version 2.2 of above (16.52 KB, patch)
2008-09-01 14:18 UTC, Tim Angus
Details | Diff
A diff on the diff yo (7.00 KB, patch)
2009-03-31 07:45 UTC, Sam Lantinga
Details | Diff
Parts missing from subversion commit of above (2.98 KB, patch)
2009-04-02 08:35 UTC, Tim Angus
Details | Diff
s/_WIN32_WCE/SDL_VIDEO_DRIVER_GAPI/ (509 bytes, patch)
2009-04-13 02:58 UTC, Tim Angus
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Angus 2008-08-12 11:18:06 UTC
[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.
Comment 1 Tim Angus 2008-08-12 11:20:34 UTC
Created attachment 263 [details]
Patch that fixes poor win32 mouse input

Bah, copying and pasting that didn't work out so well. Anyway, here is the patch.
Comment 2 Tim Angus 2008-08-12 11:22:33 UTC
*** Bug 265 has been marked as a duplicate of this bug. ***
Comment 3 Tim Angus 2008-08-30 12:15:09 UTC
Created attachment 267 [details]
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.
Comment 4 Tim Angus 2008-08-31 03:03:52 UTC
Created attachment 269 [details]
Version 2.1 of above

Fix a stupid bug relating to focus with the windib driver.
Comment 5 Gregory Smith 2008-08-31 20:40:39 UTC
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
Comment 6 Tim Angus 2008-09-01 01:55:20 UTC
Can you provide a test case?
Comment 7 Gregory Smith 2008-09-01 04:59:23 UTC
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.
Comment 8 Tim Angus 2008-09-01 14:18:49 UTC
Created attachment 271 [details]
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 :).
Comment 9 Gregory Smith 2008-09-02 19:37:39 UTC
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?
Comment 10 Tim Angus 2008-09-03 02:40:00 UTC
(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.

Comment 11 Gregory Smith 2008-09-03 07:04:34 UTC
(In reply to comment #10)
> Speaking of which how does it fail? It just doesn't start at all?

Segfaults in SDL_SetGammaRamp, IIRC.
Comment 12 Tim Angus 2008-09-03 13:24:47 UTC
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?
Comment 13 Gregory Smith 2008-09-03 13:47:19 UTC
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.
Comment 14 Tim Angus 2008-09-03 14:53:27 UTC
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).
Comment 15 Gregory Smith 2008-09-03 16:59:30 UTC
Sure enough, your DLL works. The SDL-1.2.13 DLL does not. Static builds are just the opposite.

I am truly confused.
Comment 16 Tim Angus 2008-09-04 01:51:30 UTC
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 :).
Comment 17 Gregory Smith 2008-09-04 04:35:27 UTC
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.
Comment 18 Gregory Smith 2008-09-04 18:00:06 UTC
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.
Comment 19 Sam Lantinga 2009-03-31 07:45:49 UTC
Created attachment 311 [details]
A diff on the diff yo
Comment 20 Sam Lantinga 2009-04-01 21:44:37 UTC
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?
Comment 21 Tim Angus 2009-04-02 08:35:09 UTC
Created attachment 316 [details]
Parts missing from subversion commit of above
Comment 22 Tim Angus 2009-04-02 08:45:52 UTC
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.
Comment 23 Sam Lantinga 2009-04-12 17:10:28 UTC
I'll take a look, thanks for the well organized patches and bug reports!
Comment 24 Sam Lantinga 2009-04-13 01:35:57 UTC
It looks like your diff removes some of the GAPI fixes.  Was this intentional?
Comment 25 Sam Lantinga 2009-04-13 01:38:31 UTC
Ah, I see the comment that addressed that.

Patch committed, thanks!
Comment 26 Tim Angus 2009-04-13 02:57:11 UTC
(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.
Comment 27 Tim Angus 2009-04-13 02:58:11 UTC
Created attachment 318 [details]
s/_WIN32_WCE/SDL_VIDEO_DRIVER_GAPI/

Last one, I promise!
Comment 28 Tim Angus 2009-05-07 05:46:03 UTC
Looks like this was addressed in r4508. Thanks, closing.