On Thu, Sep 6, 2018 at 12:12 AM Tobita, Tatsunosuke
<tatsunosuke.tobita@xxxxxxxxx> wrote:
>
> Hi Benjamin,
>
> Thanks I got your point. I agree with you that it is unnecessary to set anything on a digitizer which is not
> distinguished neither integrated nor external.
>
> I will make another patch without setting "INPUT_PROP_POINTER".
>
> Thanks,
>
> Tats
>
> -----Original Message-----
> From: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxx>
> Sent: Thursday, September 6, 2018 1:24 AM
> To: Jiri Kosina <jikos@xxxxxxxxxx>
> Cc: junkpainting@xxxxxxxxx; linux-input <linux-input@xxxxxxxxxxxxxxx>; Tobita, Tatsunosuke <tatsunosuke.tobita@xxxxxxxxx>; Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>; Jason Gerecke <killertofu@xxxxxxxxx>; Ping Cheng <pinglinux@xxxxxxxxx>
> Subject: Re: [PATCH] Input: Set INPUT_PROP_-property for HID_UP_DIGITIZERS
>
> On Wed, Sep 5, 2018 at 5:01 PM Jiri Kosina <jikos@xxxxxxxxxx> wrote:
> >
> > On Wed, 8 Aug 2018, Tatsunosuke Tobita wrote:
> >
> > > Some system may want to know if a detected digitizer device is
> > > either an integrated or an external device.
> > > In order to distinguish such condition, setting either
> > > INPUT_PROP_DIRECT or INPUT_PROP_POINTER is required, checking the
> > > member, "application", in "hid_field" structure.
> > >
> > > Signed-off-by: Tatsunosuke Tobita <tobita.tatsunosuke@xxxxxxxxxxx>
> > > ---
> > > drivers/hid/hid-input.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index
> > > 930652c..3e98537 100644
> > > --- a/drivers/hid/hid-input.c
> > > +++ b/drivers/hid/hid-input.c
> > > @@ -758,6 +758,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> > > break;
> > >
> > > case HID_UP_DIGITIZER:
> > > + if ((field->application & 0xff) == 0x01) /* Digitizer */
> > > + __set_bit(INPUT_PROP_POINTER, input->propbit);
> > > + else if ((field->application & 0xff) == 0x02) /* Pen */
> > > + __set_bit(INPUT_PROP_DIRECT, input->propbit);
>
> The one thing I am worried here is that the specification for "pen"
> usage states:
> "A digitizer with an integrated display that allows use of a stylus.
> The system must ensure that the sensed stylus position and the display representations of that position are the same. A pen digitizer has enough time and space resolution for handwriting input. "
> This is good, its corresponds to the patch.
>
> However, it then says in the same paragraph: "A digitizer that *may or may not* be in an integrated display application should use the more generic Digitizer collection usage. "
>
> So as I read it, "Pen" implies PROP_DIRECT, but "Digitizer" doesn't imply INPUT_PROP_POINTER.
>
> This might be the usage, so I don't know if am I just bikeshedding or if I am raising a valid point...
>
> Cheers,
> Benjamin
>
That seems like a valid observation... My initial reaction was that
not setting INPUT_PROP_POINTER would be problematic for non-display
tablets: there's little harm in showing a pointer if a "Digitizer" is
actually integrated into a display, but serious usability issues if a
pointer is not shown for a non-display tablet. As long as *no*
properties are set, however, that's not really an issue -- that just
means it's userspace's job to figure out properties, and userspace
should err on the side of showing a pointer unless something else
trips it up.
I think I'm happy with putting my Reviewed-by stamp on it either way.
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
> > > +
> > > switch (usage->hid & 0xff) {
> > > case 0x00: /* Undefined */
> > > goto ignore;
> >
> > Makes sense.
> >
> > Queued for 4.20, thanks.
> >
> > --
> > Jiri Kosina
> > SUSE Labs
> >
