linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] HID: input: fix the incorrectly reported BTN_TOOL_RUBBER/PEN tools
@ 2021-10-22 23:28 Ping Cheng
  2021-10-27 19:06 ` Ping Cheng
  0 siblings, 1 reply; 3+ messages in thread
From: Ping Cheng @ 2021-10-22 23:28 UTC (permalink / raw)
  To: jikos
  Cc: linux-input, Jason.Gerecke, tatsunosuke.tobita, Ping Cheng,
	stable, Jason Gerecke, Tatsunosuke Tobita

The HID_QUIRK_INVERT caused BTN_TOOL_RUBBER events were reported at the
same time as events for BTN_TOOL_PEN/PENCIL/etc, if HID_QUIRK_INVERT
was set by a stylus' sideswitch. The reality is that a pen can only be
a stylus (writing/drawing) or an eraser, but not both at the same time.
This patch makes that logic correct.

CC: stable@vger.kernel.org # 2.4+
Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
Reviewed-by: Jason Gerecke <killertofu@gmail.com>
Tested-by: Tatsunosuke Tobita <junkpainting@gmail.com>
---
 drivers/hid/hid-input.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 4b5ebeacd283..85741a2d828d 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1344,12 +1344,28 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 	}
 
 	if (usage->hid == HID_DG_INRANGE) {
+		/* when HID_QUIRK_INVERT is set by a stylus sideswitch, HID_DG_INRANGE could be
+		 * for stylus or eraser. Make sure events are only posted to the current valid tool:
+		 * BTN_TOOL_RUBBER vs BTN_TOOL_PEN/BTN_TOOL_PENCIL/BTN_TOOL_BRUSH/etc since a pen
+		 * can not be used as a stylus (to draw/write) and an erasaer at the same time
+		 */
+		static unsigned int last_code = 0;
+		unsigned int code = (*quirks & HID_QUIRK_INVERT) ? BTN_TOOL_RUBBER : usage->code;
 		if (value) {
-			input_event(input, usage->type, (*quirks & HID_QUIRK_INVERT) ? BTN_TOOL_RUBBER : usage->code, 1);
-			return;
+			if (code != last_code) {
+				/* send the last tool out before allow the new one in */
+				if (last_code)
+					input_event(input, usage->type, last_code, 0);
+				input_event(input, usage->type, code, 1);
+			}
+			last_code = code;
+		} else {
+			/* only send the last valid tool out */
+			if (last_code)
+				input_event(input, usage->type, last_code, 0);
+			/* reset tool for next cycle */
+			last_code = 0;
 		}
-		input_event(input, usage->type, usage->code, 0);
-		input_event(input, usage->type, BTN_TOOL_RUBBER, 0);
 		return;
 	}
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: input: fix the incorrectly reported BTN_TOOL_RUBBER/PEN tools
  2021-10-22 23:28 [PATCH] HID: input: fix the incorrectly reported BTN_TOOL_RUBBER/PEN tools Ping Cheng
@ 2021-10-27 19:06 ` Ping Cheng
  2021-10-30  2:54   ` Ping Cheng
  0 siblings, 1 reply; 3+ messages in thread
From: Ping Cheng @ 2021-10-27 19:06 UTC (permalink / raw)
  To: jikos, Dmitry Torokhov, Benjamin Tissoires
  Cc: linux-input, stable, Jason Gerecke, Tatsunosuke Tobita

Hi @Dmitry Torokhov and @Benjamin Tissoires,

Do you have any comments about this patch? The issue and the logic
behind the fix has been explained in the commit comment and in the
code. Jiri is probably waiting for an acknowledgment from one of
you...

Thank you,
Ping

On Fri, Oct 22, 2021 at 4:29 PM Ping Cheng <pinglinux@gmail.com> wrote:
>
> The HID_QUIRK_INVERT caused BTN_TOOL_RUBBER events were reported at the
> same time as events for BTN_TOOL_PEN/PENCIL/etc, if HID_QUIRK_INVERT
> was set by a stylus' sideswitch. The reality is that a pen can only be
> a stylus (writing/drawing) or an eraser, but not both at the same time.
> This patch makes that logic correct.
>
> CC: stable@vger.kernel.org # 2.4+
> Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> Reviewed-by: Jason Gerecke <killertofu@gmail.com>
> Tested-by: Tatsunosuke Tobita <junkpainting@gmail.com>
> ---
>  drivers/hid/hid-input.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 4b5ebeacd283..85741a2d828d 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1344,12 +1344,28 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
>         }
>
>         if (usage->hid == HID_DG_INRANGE) {
> +               /* when HID_QUIRK_INVERT is set by a stylus sideswitch, HID_DG_INRANGE could be
> +                * for stylus or eraser. Make sure events are only posted to the current valid tool:
> +                * BTN_TOOL_RUBBER vs BTN_TOOL_PEN/BTN_TOOL_PENCIL/BTN_TOOL_BRUSH/etc since a pen
> +                * can not be used as a stylus (to draw/write) and an erasaer at the same time
> +                */
> +               static unsigned int last_code = 0;
> +               unsigned int code = (*quirks & HID_QUIRK_INVERT) ? BTN_TOOL_RUBBER : usage->code;
>                 if (value) {
> -                       input_event(input, usage->type, (*quirks & HID_QUIRK_INVERT) ? BTN_TOOL_RUBBER : usage->code, 1);
> -                       return;
> +                       if (code != last_code) {
> +                               /* send the last tool out before allow the new one in */
> +                               if (last_code)
> +                                       input_event(input, usage->type, last_code, 0);
> +                               input_event(input, usage->type, code, 1);
> +                       }
> +                       last_code = code;
> +               } else {
> +                       /* only send the last valid tool out */
> +                       if (last_code)
> +                               input_event(input, usage->type, last_code, 0);
> +                       /* reset tool for next cycle */
> +                       last_code = 0;
>                 }
> -               input_event(input, usage->type, usage->code, 0);
> -               input_event(input, usage->type, BTN_TOOL_RUBBER, 0);
>                 return;
>         }
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] HID: input: fix the incorrectly reported BTN_TOOL_RUBBER/PEN tools
  2021-10-27 19:06 ` Ping Cheng
@ 2021-10-30  2:54   ` Ping Cheng
  0 siblings, 0 replies; 3+ messages in thread
From: Ping Cheng @ 2021-10-30  2:54 UTC (permalink / raw)
  To: jikos, Dmitry Torokhov, Benjamin Tissoires
  Cc: linux-input, stable, Jason Gerecke, Tatsunosuke Tobita

Ok, I know why no one follow-up with my patch. Life wasn't as simple
as I wished for :).

The patch missed one important fact: repeated events don't go into
userland. They are filtered! We need to initiate another input_dev to
hold all the past events for the coming tool, whether it is PEN or
RUBBER. So, it is more like a simplified multi-pen case, right? What's
your suggestions? Do we claim two tools for this particular use case?

On Wed, Oct 27, 2021 at 12:06 PM Ping Cheng <pinglinux@gmail.com> wrote:
>
> Hi @Dmitry Torokhov and @Benjamin Tissoires,
>
> Do you have any comments about this patch? The issue and the logic
> behind the fix has been explained in the commit comment and in the
> code. Jiri is probably waiting for an acknowledgment from one of
> you...
>
> Thank you,
> Ping
>
> On Fri, Oct 22, 2021 at 4:29 PM Ping Cheng <pinglinux@gmail.com> wrote:
> >
> > The HID_QUIRK_INVERT caused BTN_TOOL_RUBBER events were reported at the
> > same time as events for BTN_TOOL_PEN/PENCIL/etc, if HID_QUIRK_INVERT
> > was set by a stylus' sideswitch. The reality is that a pen can only be
> > a stylus (writing/drawing) or an eraser, but not both at the same time.
> > This patch makes that logic correct.
> >
> > CC: stable@vger.kernel.org # 2.4+
> > Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> > Reviewed-by: Jason Gerecke <killertofu@gmail.com>
> > Tested-by: Tatsunosuke Tobita <junkpainting@gmail.com>
> > ---
> >  drivers/hid/hid-input.c | 24 ++++++++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index 4b5ebeacd283..85741a2d828d 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -1344,12 +1344,28 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
> >         }
> >
> >         if (usage->hid == HID_DG_INRANGE) {
> > +               /* when HID_QUIRK_INVERT is set by a stylus sideswitch, HID_DG_INRANGE could be
> > +                * for stylus or eraser. Make sure events are only posted to the current valid tool:
> > +                * BTN_TOOL_RUBBER vs BTN_TOOL_PEN/BTN_TOOL_PENCIL/BTN_TOOL_BRUSH/etc since a pen
> > +                * can not be used as a stylus (to draw/write) and an erasaer at the same time
> > +                */
> > +               static unsigned int last_code = 0;
> > +               unsigned int code = (*quirks & HID_QUIRK_INVERT) ? BTN_TOOL_RUBBER : usage->code;
> >                 if (value) {
> > -                       input_event(input, usage->type, (*quirks & HID_QUIRK_INVERT) ? BTN_TOOL_RUBBER : usage->code, 1);
> > -                       return;
> > +                       if (code != last_code) {
> > +                               /* send the last tool out before allow the new one in */
> > +                               if (last_code)
> > +                                       input_event(input, usage->type, last_code, 0);
> > +                               input_event(input, usage->type, code, 1);
> > +                       }
> > +                       last_code = code;
> > +               } else {
> > +                       /* only send the last valid tool out */
> > +                       if (last_code)
> > +                               input_event(input, usage->type, last_code, 0);
> > +                       /* reset tool for next cycle */
> > +                       last_code = 0;
> >                 }
> > -               input_event(input, usage->type, usage->code, 0);
> > -               input_event(input, usage->type, BTN_TOOL_RUBBER, 0);
> >                 return;
> >         }
> >
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-30  2:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-22 23:28 [PATCH] HID: input: fix the incorrectly reported BTN_TOOL_RUBBER/PEN tools Ping Cheng
2021-10-27 19:06 ` Ping Cheng
2021-10-30  2:54   ` Ping Cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).