| Summary: | X11 mouse button numbering incorrect | ||
|---|---|---|---|
| Product: | SDL | Reporter: | James Legg <jlegg> |
| Component: | events | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | bb51, lukebenes, metalcaedes, rhymoid, urkle, yasushi.shoji |
| Version: | HG 2.0 | Keywords: | target-2.0.4 |
| Hardware: | x86_64 | ||
| OS: | Linux | ||
| Attachments: |
map x button values 8 and above to SDL_BUTTON_X1 and above, support vertical scrolling
Humble Bundle's patch for this issue. |
||
|
Description
James Legg
2014-03-31 09:23:43 UTC
This sounds like new X server behavior. Ryan, can you comment? I just found out this bug while testing 0AD on SDL2. What I found in hg repo is taht those values differ from day 1. - http://hg.libsdl.org/SDL/rev/2284 - http://hg.libsdl.org/SDL/rev/4113 Was there any reason to do this? Do SDL_BUTTON_X1 and SDL_BUTTON_X2 currently work under Non X Window System?
> This sounds like new X server behavior.
>
> Ryan, can you comment?
I'm not aware of a change, but it's not something I would have looked for.
--ryan.
(In reply to yasushi.shoji from comment #2) > I just found out this bug while testing 0AD on SDL2. What I found in hg > repo is taht those values differ from day 1. > > - http://hg.libsdl.org/SDL/rev/2284 > - http://hg.libsdl.org/SDL/rev/4113 > > Was there any reason to do this? Do SDL_BUTTON_X1 and SDL_BUTTON_X2 > currently work under Non X Window System? Those values are different between SDL 1.2 and 2.0, which are not meant to be API or ABI compatible. (In this case, SDL 2.0 moved mouse wheel input to its own event and stopped treating it like a mouse button, and the X11 code should take steps to deal with that...unless it's buggy or some assumption we relied on changed, of course.) --ryan. (In reply to Ryan C. Gordon from comment #4) > Those values are different between SDL 1.2 and 2.0, which are not meant to > be API or ABI compatible. OK. BTW, what does SDL_BUTTON_X1 and 2 intended to be? Does it have a fixed meaning or are they just arbitrary buttons on a mouse? it seems to me that every back-end defines them differently. > (In this case, SDL 2.0 moved mouse wheel input to its own event and stopped > treating it like a mouse button, and the X11 code should take steps to deal > with that...unless it's buggy or some assumption we relied on changed, of > course.) Would you mind if I ask what you mean? Do you mean to have #ifdef's in application code, or suggesting to modify SDL's X11 code? I don't mind SDL 1.2 vs 2.0 compatibility. But I assumed, with SDL 2.0, they have the same meaning on all supported systems. (In reply to yasushi.shoji from comment #5) > Would you mind if I ask what you mean? Do you mean to have #ifdef's in > application code, or suggesting to modify SDL's X11 code? No, these should work the same everywhere, as far as the app is concerned. It's certainly possible SDL has a bug here. We haven't seen many mice using those extra buttons. It should be reporting left, right, middle, then the wheels should be reported on a different type of event, then the next buttons should be X1 and X2. > I don't mind SDL 1.2 vs 2.0 compatibility. But I assumed, with SDL 2.0, they > have the same meaning on all supported systems. They do (or at least, they should!). --ryan. >> But I assumed, with SDL 2.0, they have the same meaning on all supported systems. > They do (or at least, they should!). They do not. Windows and Linux differ in SDL 2. Developers have encountered this bug here: http://trac.wildfiregames.com/ticket/2041 https://github.com/RobertBeckebans/RBDOOM-3-BFG/issues/89 and a possible dupe here: https://bugzilla.libsdl.org/show_bug.cgi?id=2310 Are there plans to fix this, so developers don't need a Linux #ifdef hack to support more than 3 mouse buttons? Created attachment 2026 [details]
map x button values 8 and above to SDL_BUTTON_X1 and above, support vertical scrolling
I wrote a patch that makes sure that xevent.xbutton.button value 8 yields SDL_BUTTON_X1, 9 yields SDL_BUTTON_X2 etc (by sutracting (8-SDL_BUTTON_X1) from the value, if it's > 7).
It also generates vertical scrolling events for X11 buttons 6 and 7 (using the same logic as for buttons 4 and 5 to make reasonably sure it's really a scroll event).
Note that the mouse button values are hardcoded because X.h only defines up to Button5.
It would be great if other people could test if this works for them - buttons X1 and above work great on my machine, but I couldn't test vertical scrolling, as my mouse doesn't support it.
I just discovered the following: <pre> $ xinput --list --long 2 Virtual core pointer id=2 [master pointer (3)] Reporting 5 classes: Class originated from: 8. Type: XIButtonClass Buttons supported: 16 Button labels: "Button Left" "Button Middle" "Button Right" "Button Wheel Up" "Button Wheel Down" "Button Horiz Wheel Left" "Button Horiz Wheel Right" "Button Side" "Button Extra" "Button Forward" "Button Back" "Button Task" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" (...) </pre> so the xinput program knows what button (number) is which.. whatever it does to get that information (haven't looked at the source), SDL could do the same? The button names above "Button Horiz Wheel" don't make much sense though.. X1 seems to be "Button Side", X2 "Button Extra", my strange whatever button on the mouse "Button Forward", even though I'm pretty sure that's not what it is.. But I guess the buttons after the scroll wheel buttons could just be passed to SDL in that order (just subtract the number of mousewheel buttons) (In reply to Daniel Gibson from comment #8) > Created attachment 2026 [details] > map x button values 8 and above to SDL_BUTTON_X1 and above, support vertical > scrolling Do you mean horizontal scrolling? I'm just curious because vertical or y (as I understand it) is mouse wheel up/down, while tilt would be left/right horizontal or x. I applied the patch to the latest Hg snapshot, but it didn't fix the issue I have with tilt buttons on Ubuntu 14.04 w/ SDL2. On Windows, they produce horizontal scrolling SDL_MOUSEWHEEL events as Ryan explained, which our game handles correctly. The same should happen on Linux (I think we are all in agreement on that). oh right, for some reason I always mix up vertical and horizontal :-/ It's for scrolling left/right :-) Oh, and about the issue, can you run the test program from https://bugzilla.libsdl.org/show_bug.cgi?id=2310 and tell me the output for the "tilt buttons"? Could you also tell me the output of "xinput --list --long"? (More specifically, the button labels of your actual mouse device are enough) (In reply to Daniel Gibson from comment #12) > Oh, and about the issue, can you run the test program from > https://bugzilla.libsdl.org/show_bug.cgi?id=2310 and tell me the output for > the "tilt buttons"? Tested with and without the patch, with the same results: There is normally no output for the mousewheel, but I think that's expected, since that doesn't generate SDL_MOUSEBUTTON* events anymore. One strange thing is if I scroll quickly up/down for a while, it generates a single button 4 or 5 event. > Could you also tell me the output of "xinput --list --long"? > (More specifically, the button labels of your actual mouse device are enough) Reporting 7 classes: Class originated from: 7. Type: XIButtonClass Buttons supported: 24 Button labels: "Button Left" "Button Middle" "Button Right" "Button Wheel Up" "Button Wheel Down" "Button Horiz Wheel Left" "Button Horiz Wheel Right" "Button Side" "Button Extra" "Button Forward" "Button Back" "Button Task" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" "Button Unknown" (In reply to historic_bruno from comment #13) > (In reply to Daniel Gibson from comment #12) > > Oh, and about the issue, can you run the test program from > > https://bugzilla.libsdl.org/show_bug.cgi?id=2310 and tell me the output for > > the "tilt buttons"? > > Tested with and without the patch, with the same results: > There is normally no output for the mousewheel, but I think that's expected, > since that doesn't generate SDL_MOUSEBUTTON* events anymore. Uhh.. so those tilt buttons don't generate any event at all? Neither scroll events nor button events? > One strange > thing is if I scroll quickly up/down for a while, it generates a single > button 4 or 5 event. Maybe in that case SDLs strange hack to detect if it's really a wheel event (down and up event at the same time for that mouse button) doesn't work. (The button labels look ok to me) can you run "xev -event mouse | grep -A3 Button" And look if you get any button events for your tilt buttons there? One event should look like ButtonRelease event, serial 25, synthetic NO, window 0x7a00001, root 0x29c, subw 0x0, time 981125016, (141,43), root:(1368,734), state 0x10, button 8, same_screen YES and the most interesting part is "button 8" (or whatever number you'll get) in the last line. If that command doesn't yield any output for the tilt buttons, run it again without "| grep .." and try to find output for the tilt buttons in there (unfortunately, the output is usually spammed by mouse motion events, so try to not move the mouse, for mine it works well to lift it up from the table and just press the button. just make sure the cursor is still on the xev window) Thanks for those handy debugging commands, it was a problem with my mouse driver all along! Now that is sorted out, tilting does trigger buttons 6 and 7, and it functions like horizontal scrolling with your patch. Behavior seems consistent on my Windows and Ubuntu test systems. Nicely done and sorry for the confusion :) Great! Maybe mouse buttons 4/5 should always be treated as scroll events, i.e. the check should either be relaxed or changed to use whatever xinput uses - either way, one probably doesn't want false positive button x1/x2 events when scrolling a lot. *** Bug 2310 has been marked as a duplicate of this bug. *** Marking a large number of bugs with the "triage-2.0.4" keyword at once. Sorry if you got a lot of email from this. This is to help me sort through some bugs in regards to a 2.0.4 release. We may or may not fix this bug for 2.0.4, though! (sorry if you get a lot of copies of this email, I'm marking several bugs at once) Marking bugs for the (mostly) final 2.0.4 TODO list. This means we're hoping to resolve this bug before 2.0.4 ships if possible. In a perfect world, the open bug count with the target-2.0.4 keyword is zero when we ship. (Note that closing a bug report as WONTFIX, INVALID or WORKSFORME might still happen.) --ryan. (In reply to historic_bruno from comment #13) > > Tested with and without the patch, with the same results: > There is normally no output for the mousewheel, but I think that's expected, > since that doesn't generate SDL_MOUSEBUTTON* events anymore. One strange > thing is if I scroll quickly up/down for a while, it generates a single > button 4 or 5 event. > I just observed the same behavior, btw. So it seems like the check for "is this really a button event" does indeed not work well. I guess buttons 4-7 should always (and only) generate scroll events and the buttons above button down events. or maybe instead of hardcoding the 4-7 for mouse wheel get the button names like xinput does and compare with "Button Wheel Up" "Button Wheel Down" "Button Horiz Wheel Left" "Button Horiz Wheel Right". I think this is the code that xinput uses to get the labels: http://cgit.freedesktop.org/xorg/app/xinput/tree/src/list.c#n161 it seems to require xinput2. it gets "XIAnyClassInfo **classes , int num_classes" from an "XIDeviceInfo*" (it's the "classes" and "num_classes" members). it iterates classes, and if "classes[i]->type == XIButtonClass" it can be cast to "XIButtonClassInfo*" (let's call it "b") and there's "b->labels[]" containing "b->num_buttons" entries of X Atoms, I think. the name is retrieved with "XGetAtomName(display, b->labels[j])" (if the atom is non-null) This is good to fix, but requires enough testing that it should probably wait until after 2.0.4 this means at least another year with broken mouse button 4+ handling and without support for horizontal scrolling on Linux? :-( it can't get any more broken than it currently is, why not just release a fix and see what happens? Created attachment 2168 [details]
Humble Bundle's patch for this issue.
FYI.. Humble Bundle has been using this patch for a while and in several games including:
Torchlight II, Shadow Warrior (2013), and La Mulana. All of these released in the HIB 14 bundle.
Yeah, that's my patch from above. I'm glad it's already used and works for you! :-) One thing I'd change is to relax the checks in X11_IsWheelEvent() to not check if the button has been pressed and released at the same time (sometimes that doesn't seem to happen and mouse 4/5 events are generated, see the comments here), but just *always* map buttons 4-7 to wheel events. I'm going to mark this as target-2.0.4 again, since there's a patch that's gotten widespread testing. Sam, if you want to yank the keyword again, that's cool, though. I'll play with the patch tonight otherwise. --ryan. This looks good to fix for 2.0.4. Thanks Edward and Daniel! Daniel's patch (which is the exact one Humble Bundle is using) is now https://hg.libsdl.org/SDL/rev/d84e21b0b0ba, thanks everyone! --ryan. (In reply to Daniel Gibson from comment #24) > One thing I'd change is to relax the checks in X11_IsWheelEvent() to not > check if the button has been pressed and released at the same time > (sometimes that doesn't seem to happen and mouse 4/5 events are generated, > see the comments here), but just *always* map buttons 4-7 to wheel events. We can do this for 2.0.5, but if the existing patch is working without serious complaints, I'd rather not mess with it further for 2.0.4. Open a new bug if you like so we revisit it after 2.0.4 ships. --ryan. Thanks for merging this patch for 2.0.4! :-) I'll create another bugreport for the "sometimes mousewheel is identified as X1/X2" problem. That problem seems to be rare enough (even those affected only observe it infrequently), so it's ok to wait for 2.0.5 ok, I should have looked for such a report first. It already exists: https://bugzilla.libsdl.org/show_bug.cgi?id=2987 |