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

description for SDL_MouseButtonEvent.button is misleading #1282

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

description for SDL_MouseButtonEvent.button is misleading #1282

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Labels
duplicate This issue or pull request already exists

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: 2.0.3
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2013-12-16 12:12:02 +0000, Stijn van Drongelen wrote:

See http://hg.libsdl.org/SDL/file/405d84eedad8/src/video/x11/SDL_x11events.c#l711:

case ButtonPress:{
        int ticks = 0;
        if (X11_IsWheelEvent(display,&xevent,&ticks)) {
            SDL_SendMouseWheel(data->window, 0, 0, ticks);
        } else {
            SDL_SendMouseButton(data->window, 0, SDL_PRESSED, xevent.xbutton.button);
        }
    }
    break;

case ButtonRelease:{
        SDL_SendMouseButton(data->window, 0, SDL_RELEASED, xevent.xbutton.button);
    }
    break;

In the case of my own Logitech M570 mouse, which apparently has 20 buttons, X calls my "up" and "down" button 8 and 9. Because SDL blindly copies these IDs, the documentation of SDL_MouseButtonEvent.button ("may be one of SDL_BUTTON_*") is now meaningless. Users who depend on this to mean that button can only be one of those five values (e.g. the Haskell bindings for SDL2, a work in progress) will end up crashing.

I suppose the documentation needs to reflect that .button may be something else than described in SDL_BUTTON_*.

On 2013-12-19 13:23:30 +0000, Turo Lamminen wrote:

The behavior is also different between platforms. I have a Logitech MX518 with three extra buttons (back, forward and home). SDL 2.0.1 reports them as follows:

Windows reports back & forward as 4 & 5 as documentation says. Home is not reported at all.

Mac OS X 10.8 reports back & forward as 4 & 5 and home as 6. Documentation is misleading.

Linux (Debian testing) reports back & forward as 8 & 9 and home as 10. Totally different from both documentation and also from other platforms.

On 2015-01-27 17:29:03 +0000, Daniel Gibson wrote:

I can confirm this behavior, on both Linux Mint 17.1 KDE and Debian Wheezy with XFCE. Also with two different mice: Logitech MX518 and some cheap Gigabyte Mouse).
I tested with SDL2 2.0 (I think) on Wheezy and 2.0.2 and latest HG (9319:ad4f430cc9f5) on Mint.

Both report the "forward"/"back" buttons (aka X1 and X2) as 8 and 9.
One additional button on the MX518 (zoom or something?) is reported as 10.

I think it's an actual bug that X1 and X2 are not reported as SDL_BUTTON_X1 and SDL_BUTTON_X2 in the SDL_MouseButtonEvent.

I have no idea why the "Importance" was set to "API change" - the API docs may be unclear on what happens when pressing a mouse button > X2, but documenting that wouldn't change/break the API - and neither would fixing this bug.
Thus I changed it to "major".

From the code it's not surprising that this happens: In
SDL_x11events.c "static void X11_DispatchEvent(_THIS)", "case ButtonPress:"
there is some mousewheel magic that I haven't looked closely at and then:
SDL_SendMouseButton(data->window, 0, SDL_PRESSED, xevent.xbutton.button);
(Similar for ButtonRelease)

So the xevent.xbutton.button value is just used unchanged to generate the SDL event.
But AFAIK X11 buttons 4 and 5 are used for horizontal scrolling and 6 and 7 for vertical scrolling - so it kinda makes sense that the next "real" (non-wheel) button gets the value 8.
Not sure if X11 always assigns it like this or only with [XYZ]AxisMapping enabled...

Apart from this problem, the documentation should indeed be more clear about SDL_MouseButtonEvent.button's value that can not only take the given constants but can be > SDL_BUTTON_X2 (I guess everything up to 32 or so should be ok for the SDL_BUTTON() macro/ SDL_GetMouseState()).

On 2015-01-27 17:31:53 +0000, Daniel Gibson wrote:

Created attachment 2006
Simple test to print the mouse button number

On 2015-02-01 13:08:23 +0000, Stijn van Drongelen wrote:

I have no idea why the "Importance" was set to "API change" - the API
docs may be unclear on what happens when pressing a mouse button >
X2, but documenting that wouldn't change/break the API - and neither
would fixing this bug. Thus I changed it to "major".

In my opinion, updating the documentation of a C library does change
the the API, because the documentation is the API. There is nothing
on the language level to give a complete and thorough description of
the API, so you have to document it separately to even have an API
in the first place.

I don't mean to start a debate on "what is an API", but I think that
I saw that importance tag a year ago and thought it was fitting for
the reason stated above.

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

Well, that is not completely unreasonable, but mapping the events to the existing SDL_BUTTON_X1/2 constants would at least fix most of this bug and comply with the API (in contrast to the current behavior).

I can however imagine that the maintainers ignored this bug because they assumed this issue demanded breaking the API and didn't want to break the API anytime soon.

In this particular case I'd prefer "API breakage" by mentioning that more buttons can be supported but just don't have predefined constants vs waiting for the next major API-breaking SDL release to support more than 5 buttons.
Especially because the implementation already does that anyway and it doesn't change any function interfaces or anything.

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

Possible dupe here:
https://bugzilla.libsdl.org/show_bug.cgi?id=2472

On 2015-02-02 07:50:22 +0000, Daniel Gibson wrote:

Yeah, pretty sure it's the same bug.

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

Marking this as a dupe of Bug # 2472 (which has more discussion, too).

*** This bug has been marked as a duplicate of bug 2472 ***

@SDLBugzilla SDLBugzilla added bug duplicate This issue or pull request already exists labels Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

1 participant