Re: [RFC 1/6] Input: Rename extra mouse buttons defines to match their actual usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dmitry,

On 04-04-19 03:00, Dmitry Torokhov wrote:
Hi Hans,

On Wed, Apr 03, 2019 at 05:37:11PM +0200, Hans de Goede wrote:
Hi,

On 03-04-19 16:12, Benjamin Tissoires wrote:
On Mon, Apr 1, 2019 at 1:11 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:

Hi,

On 01-04-19 12:34, Peter Hutterer wrote:
On Mon, Apr 01, 2019 at 09:16:22AM +0200, Hans de Goede wrote:
Hi,

On 01-04-19 05:05, Peter Hutterer wrote:
On Sat, Mar 30, 2019 at 02:37:30PM +0100, Hans de Goede wrote:
Almost all modern mice use HID. For buttons 1 - 3 the order of left /
right / middle specified in the HUT matches with the BTN_LEFT, BTN_RIGHT
and BTN_MIDDLE order of the evdev code defines since the HUT does not
specify any meaning for button usages >= 4 the HID input driver simply
maps buttons to BTN_LEFT + (number - 1).

In practice on most mice which have more then 3 buttons, usage 4 is
used for the back(ward) button and usage 5 for the forward button.

This means that these buttons generate an input_event code of 0x113 resp.
0x114, which looking at the old defines maps to BTN_SIDE and BTN_EXTRA.

Under X these are mapped to buttons 8 resp. 9; and under wayland the
input_event code is passed through unmodified. Apps, e.g. Firefox both
when running as Wayland client under GNOME3 and when running under Xorg,
correctly interpret these as back and forward.

So we've back and forward buttons generating BTN_SIDE and BTN_EXTRA,
which the apps then interpret as BTN_BACK and BTN_FORWARD, rather then as
BTN_SIDE and BTN_EXTRA (which have no clear meaning).

I find this all very confusing, this commit tries to remove the confusion
by deprecating the old defines and adding new defines which assign labels
to the 0x113 - 0x116 input_event codes which match how they are actually
used today.

Note there are no functional changes here, after this userspace will
see the exact same input_event codes as before, this purely about
assigning a human-readable label to these codes which matches with how
they are actually being used.
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
     Documentation/input/event-codes.rst    |  2 +-
     drivers/hid/hid-debug.c                |  6 +++---
     drivers/input/mousedev.c               |  8 ++++----
     include/uapi/linux/input-event-codes.h | 13 ++++++++++++-
     4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
index b24b5343f5eb..db52b96e7a83 100644
--- a/Documentation/input/event-codes.rst
+++ b/Documentation/input/event-codes.rst
@@ -422,7 +422,7 @@ should be used to report when the tool is in contact with the tablet.
     BTN_{STYLUS,STYLUS2} should be used to report buttons on the tool itself. Any
     button may be used for buttons on the tablet except BTN_{MOUSE,LEFT}.
     BTN_{0,1,2,etc} are good generic codes for unlabeled buttons. Do not use
-meaningful buttons, like BTN_FORWARD, unless the button is labeled for that
+meaningful buttons, like BTN_FORWRD, unless the button is labeled for that

I haven't thought of a better name yet, but using BTN_FORWRD is bound to
introduce bugs, especially when BTN_FORWARD still resolves. I don't think
that name is a good idea but I don't have a better suggestion yet.

Good point about the typos my plan for the kernel is to add #ifndef __KERNEL__
around the compat defines once all users are moved over.

good, but not enough. the kernel is easiest here because you could literally
just add a script to a test suite to make sure it doesn't use BTN_FORWARD.
It's all the userspace that'll introduce typos.

Thought of the day: if HID doesn't have any meaning, is it really worth
changing these names based on what you hope a client interprets it as?
would a more generic naming scheme that admits this is just button 4 be
better? BTN_HID_4 or something?

The HUT spec may not say anything about this (note I did not check the
gazillion errata I really wish they would just fold those into an
updated version), but Microsoft writes the following about this and it
is reasonable to expect mice vendors to follow this (and in practice they do):
https://docs.microsoft.com/en-us/windows-hardware/drivers/hid/keyboard-and-mouse-hid-client-drivers

"Buttons 4 & 5 on are mapped to WM_APPCOMMAND messages and correspond to App_Back and App_Forward."

BTN_APP_FORWARD and BTN_APP_BACK then? seems obvious enough and is a lot
easier to visually identify than a missing A in FORWRD.

Yes I was thinking the same about 5 minutes after sending the mail you
replied to, so I think we agree on this, will fix for the the first non
RFC version.

For the record, I also agree with BTN_APP_FORWARD and BTN_APP_BACK.

We made some archeology this morning with Peter.
So now is story time, gather around children:
BTN_SIDE and BTN_EXTRA were added in 2.3.36 with patch-2.3.36.bz2 that
can be found at
https://mirrors.edge.kernel.org/pub/linux/kernel/v2.3/.

It's actually the big switch from Vojtech from a custom USB mouse boot
protocol to an actual HID bus. It's also the creation of the evdev
protocol.

And those BTN_* were added to suport mice with extra buttons. I guess
it was pretty uncommon for such mice in Jan 2000 and the buttons were
not commonly used like today as back and forward.

Note that it's actually easy to go back to 2.4.0 by using:
https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git/log/?ofs=63400

So, my fears that the buttons would have a meaning is relieved, and I
would Ack such a series (I haven't fully read it so it's an ACK on the
principle).

Ok, so before I send a v1 of this patch-set, Dmitry do you have any remarks?

I played with my Evoluent Vertical mouse and I found that it reports
BTN_EXTRA for button under scroll wheel, BTN_SIDE for the button on the
side of the mouse and BTN_FORWARD for the 2nd button on the side. This
means that not all mice have these buttons in forward/back positions...

Well that mouse seems to be quite special, most mice with more then 3
buttons have the back/forward side buttons as buttons 4 and 5.

Although I myself in the meantime have found a no-name bluetooth mouse
which has button 4 forward button 5 back (so swapped compared to what
userspace expects).

I will try to borrow some mice with more then 3 buttons from people at
my local hackerspace and see if there is some consistency there,
at least the microsoft page I pointed to suggests that what my logitech
mice are doing is the default Windows behavior.

And I just plugged the receiver for these Logitech mice into a win10
machine and without installing any logitech drivers, the buttons
correctly do back / forward in the webbrowser there.

I also tried the bt mouse with the swapped buttons with win10 and
indeed the buttons work in the wrong direction there, so that seems
to be a hardware-misfeature of that mouse.

So it still seems to me that the default Linux userspace behavior
(at least in Firefox) with button 4 is back and button 5 is forward
is correct.

Still I find the existing BTN_FORWARD and BTN_BACK defines misleading
and as e.g. the m560 code in drivers/hid/hid-logitech-hidpp.c shows
various drivers to try to use them as if userspace will actually
interpret them the way the name suggests, which it typically will not.

So maybe just rename all 5 defines for buttons 4-8 to BTN_EXTRA1 - BTN_EXTA5?

With maybe a comment that userspace may interpret BTN_EXTRA4 as back and
BTN_EXTRA5 as forward (or maybe even BTN_APP_FORWARD and BTN_APP_BACK aliases?) ?

And then as Peter suggested in another thread about other extra mouse buttons
(on which you were not Cc-ed) we really need userspace to have a mapping UI
for special / broken cases.

Regards,

Hans



[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux