Re: [PATCH 1/2] HID: multitouch: report MT_TOOL_PALM for non-confident touches

From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Henrik Rydberg <rydberg@bitmath.org>,
	Jiri Kosina <jikos@kernel.org>,
	Jason Gerecke <killertofu@gmail.com>,
	Dennis Kempin <denniskempin@google.com>,
	Andrew de los Reyes <adlr@google.com>,
	"open list:HID CORE LAYER" <linux-input@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] HID: multitouch: report MT_TOOL_PALM for non-confident touches
Date: Mon, 4 Jun 2018 22:59:16 +0200	[thread overview]
Message-ID: <CAO-hwJK3h8xjuO=GQRT0prdbXURCTinkQg4i7N8ScPOig1KTYw@mail.gmail.com> (raw)
In-Reply-To: <20180604182658.GC164893@dtor-ws>

On Mon, Jun 4, 2018 at 8:26 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Jun 04, 2018 at 07:55:57PM +0200, Henrik Rydberg wrote:
>> Hi Dmitry,
>>
>> > > > Logically, the confidence state is a property of a contact, not a new type
>> > > > of contact. Trying to use it in any other way is bound to lead to confusion.
>> > > >
>> > > > Problem is that MT_TOOL_PALM has been introduced in the kernel since
>> > > > v4.0 (late 2015 by a736775db683 "Input: add MT_TOOL_PALM").
>> > > > It's been used in the Synaptics RMI4 driver since and by hid-asus in late 2016.
>> > > > I can't find any other users in the current upstream tree, but those
>> > > > two are already making a precedent and changing the semantic is a
>> > > > little bit late :/
>> > I am sorry I did not respond and lost track of this issue back then, but
>> > I disagree with Henrik here. While confidence is a property of contact,
>> > so is the type of contact and it can and will change throughout life of
>> > a contact, especially if we will continue adding new types, such as, for
>> > example, thumb. In this case the firmware can transition through
>> > finger->thumb or finger->thumb->palm or finger->palm as the nature of
>> > contact becomes better understood. Still it is the same contact and we
>> > should not attempt to signal userspace differently.
>> We agree that the contact should stay the same, but the fear, and I think
>> somewhere along the blurry history of this thread, the problem was that
>> userspace interpreted the property change as a new contact (lift up/double
>> click/etc). Finger/thumb/palm is one set of hand properties, but what about
>> Pen? It would be hard for an application to consider a switch from finger to
>> pen as the same contact, which is where the natural implementation starts to
>> diverge from the intention.
>
> I think the userspace has to trust our tracking ID to decide whether it
> is a same contact or not. The current issue is that kernel is forcing
> tracking ID change on tool type change, and one of the 2 patches that I
> posted fixed that, allowing us to keep the tracking ID for finger->palm
> transitions.

I think I missed those 2 patches, can you point a LKML link?
Also, note that libevdev discards the tracking ID change now (it
shouts at the user in the logs). So that means that it will now be
hard to force libevdev to trust the kernel again for the tracking ID.
The current rule is:
- tracking ID >= 0 -> new touch
- any subsequent tracking ID >= 0 -> discarded
- tracking ID == -1 -> end of touch

>
> I think it is kernel task to not signal transitions that do not make
> sense, such as finger->pen or palm->pen etc.

I fully agree, though there is currently no such guard in the kernel
(maybe it's part of your series). I am worried about the RMI4 F12
driver that automatically forward the info from the firmware, so if
the firmware does something crazy, it will be exported to user space.
But I guess it might be better to treat that on a per driver basis.

>
>>
>> > We could introduce the ABS_MT_CONFIDENCE (0/1 or even 0..n range), to
>> > complement ABS_MT_TOOL, but that would not really solve the issue with
>> > Wacom firmware (declaring contact non-confident and releasing it right
>> > away) and given MS explanation of the confidence as "contact is too big"
>> > MT_TOOL_PALM fits it perfectly.
>> Indeed, the Wacom firmware seems to need some special handling, which should
>> be fine by everyone. I do think it would make sense to add
>> ABS_MT_TOOL_TOO_BIG, or something, and use it if it exists. This would apply

Except we are already running out of ABS_* axes.

>> also to a pen lying down on a touchpad, for instance.
>
> OK, I can see that for Pens, if we have firmware that would recognize
> such condition, it would be weird to report PALM. We could indeed have
> ABS_MT_TOOL_TOO_BIG, but on the other hand it is still a pen (as long as
> the hardware can recognize it as such). Maybe we'd be better off just
> having userspace going by contact size for pens. Peter, any suggestions
> here?

I don't think we have size handling in the tablet implementation in
libinput. I do not see it as a big issue to add such axes from a
libinput point of view. However, there is no existing hardware that
would provide such information, so I guess this will be a 'no' until
actual hardware comes in.

Also note that the MT_TOOL_PEN implementation is limited (even
non-existant if I remember correctly). Peter and I do not have access
to any device that support such multi pen, so AFAICT, there is no code
to handle this in libinput.

One last point from libinput, the pen device would need to be on its
separate kernel node for the protocol to be smoothly handled. So
basically, even the transition from MT_TOOL_FINGER to MT_TOOL_PEN
would not be handled properly right now. The Pen event will be treated
as a touch.

Cheers,
Benjamin

>
> Thanks.
>
> --
> Dmitry