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

X11 mouse button numbering incorrect #1421

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

X11 mouse button numbering incorrect #1421

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 10, 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: Linux, x86_64

Comments on the original bug report:

On 2014-03-31 09:23:43 +0000, James Legg wrote:

Under X11, the forward and backwards buttons on the side of a 5 button mouse produce SDL_MouseButtonEvents with the button member set to 8 or 9. I think that button should be set to SDL_BUTTON_X1 and SDL_BUTTON_X2 for these events. I get 4 (SDL_BUTTON_X1) and 5 (SDL_BUTTON_X2) instead of 8 and 9 when using the same mouse on OS X.

Note that on X11, mouse buttons 4 and 5 correspond to vertical scrolling, and 6 and 7 are for horizontal scrolling. Buttons additional to the traditional three are therefore numbered from 8 instead of 4.

On 2014-04-17 09:31:48 +0000, Sam Lantinga wrote:

This sounds like new X server behavior.

Ryan, can you comment?

On 2014-09-09 00:19:01 +0000, wrote:

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.

Was there any reason to do this? Do SDL_BUTTON_X1 and SDL_BUTTON_X2 currently work under Non X Window System?

On 2014-09-09 01:58:59 +0000, Ryan C. Gordon wrote:

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.

On 2014-09-09 02:00:55 +0000, Ryan C. Gordon wrote:

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

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.

On 2014-09-09 10:15:40 +0000, wrote:

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

On 2014-09-16 04:18:06 +0000, Ryan C. Gordon wrote:

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

On 2015-02-01 20:54:17 +0000, Luke wrote:

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
RobertBeckebans/RBDOOM-3-BFG#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?

On 2015-02-09 03:18:45 +0000, Daniel Gibson wrote:

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

On 2015-02-12 03:18:13 +0000, Daniel Gibson wrote:

I just discovered the following:

$ 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"
(...)

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)

On 2015-02-18 00:08:58 +0000, historic_bruno wrote:

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

On 2015-02-18 00:28:12 +0000, Daniel Gibson wrote:

oh right, for some reason I always mix up vertical and horizontal :-/

It's for scrolling left/right :-)

On 2015-02-18 00:31:25 +0000, Daniel Gibson wrote:

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)

On 2015-02-18 01:40:12 +0000, historic_bruno wrote:

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

On 2015-02-18 01:55:32 +0000, Daniel Gibson wrote:

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

On 2015-02-18 02:34:06 +0000, historic_bruno wrote:

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

On 2015-02-18 03:44:07 +0000, Daniel Gibson wrote:

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.

On 2015-02-19 05:30:52 +0000, Ryan C. Gordon wrote:

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

On 2015-02-19 06:32:16 +0000, Ryan C. Gordon wrote:

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!

On 2015-04-07 04:57:55 +0000, Ryan C. Gordon wrote:

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

On 2015-04-13 02:30:25 +0000, Daniel Gibson wrote:

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

On 2015-05-28 17:48:22 +0000, Sam Lantinga wrote:

This is good to fix, but requires enough testing that it should probably wait until after 2.0.4

On 2015-05-28 18:52:45 +0000, Daniel Gibson wrote:

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?

On 2015-05-29 15:11:22 +0000, Edward Rudd wrote:

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

On 2015-05-29 15:18:01 +0000, Daniel Gibson wrote:

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.

On 2015-05-29 19:45:47 +0000, Ryan C. Gordon wrote:

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.

On 2015-05-29 20:14:32 +0000, Sam Lantinga wrote:

This looks good to fix for 2.0.4. Thanks Edward and Daniel!

On 2015-05-31 04:59:47 +0000, Ryan C. Gordon wrote:

Daniel's patch (which is the exact one Humble Bundle is using) is now https://hg.libsdl.org/SDL/rev/d84e21b0b0ba, thanks everyone!

--ryan.

On 2015-05-31 05:02:23 +0000, Ryan C. Gordon wrote:

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

On 2015-05-31 13:39:47 +0000, Daniel Gibson wrote:

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

On 2015-05-31 13:41:28 +0000, Daniel Gibson wrote:

ok, I should have looked for such a report first. It already exists: https://bugzilla.libsdl.org/show_bug.cgi?id=2987

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

1 participant