* [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
@ 2012-02-21 15:07 Chung-yih Wang
2012-02-21 16:35 ` Daniel Kurtz
0 siblings, 1 reply; 13+ messages in thread
From: Chung-yih Wang @ 2012-02-21 15:07 UTC (permalink / raw)
To: Alessandro Rubini
Cc: Dmitry Torokhov, linux-input, linux-kernel, djkurtz,
Chung-yih Wang
The single touch path imposes a minimum z value (with hysterisis) before
registering BTN_TOUCH. Apply the same (hard coded) threshold when
deciding how many fingers to report in the semi-mt path.
This patch improves performance of the Google Cr-48 chromebook's
extremely sensitive Synaptics profile sensor touchpad by filtering out
touch events for hovering fingers.
Note: We continue to use the same hard coded threshold value used in the
single touch case as it appears this works just as well on these
multitouch profile sensor pads as on whatever pads it was originally
discovered.
Signed-off-by: Chung-Yih Wang <cywang@chromium.org>
---
drivers/input/mouse/synaptics.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 8081a0a..7936a1a 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -568,17 +568,22 @@ static void synaptics_report_semi_mt_slot(struct input_dev *dev, int slot,
}
}
+static bool finger_touched(const struct synaptics_hw_state *hw)
+{
+ return (hw->z > 30);
+}
+
static void synaptics_report_semi_mt_data(struct input_dev *dev,
const struct synaptics_hw_state *a,
const struct synaptics_hw_state *b,
int num_fingers)
{
- if (num_fingers >= 2) {
+ if ((num_fingers >= 2) && finger_touched(a) && finger_touched(b)) {
synaptics_report_semi_mt_slot(dev, 0, true, min(a->x, b->x),
min(a->y, b->y));
synaptics_report_semi_mt_slot(dev, 1, true, max(a->x, b->x),
max(a->y, b->y));
- } else if (num_fingers == 1) {
+ } else if ((num_fingers == 1) && finger_touched(a)) {
synaptics_report_semi_mt_slot(dev, 0, true, a->x, a->y);
synaptics_report_semi_mt_slot(dev, 1, false, 0, 0);
} else {
@@ -1040,7 +1045,8 @@ static void synaptics_process_packet(struct psmouse *psmouse)
* BTN_TOUCH has to be first as mousedev relies on it when doing
* absolute -> relative conversion
*/
- if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
+ if (finger_touched(&hw))
+ input_report_key(dev, BTN_TOUCH, 1);
if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
if (num_fingers > 0) {
--
1.7.7.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-21 15:07 Chung-yih Wang
@ 2012-02-21 16:35 ` Daniel Kurtz
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Kurtz @ 2012-02-21 16:35 UTC (permalink / raw)
To: Chung-yih Wang
Cc: Alessandro Rubini, Dmitry Torokhov, linux-input, linux-kernel
On Tue, Feb 21, 2012 at 11:07 PM, Chung-yih Wang <cywang@chromium.org> wrote:
> The single touch path imposes a minimum z value (with hysterisis) before
> registering BTN_TOUCH. Apply the same (hard coded) threshold when
> deciding how many fingers to report in the semi-mt path.
>
> This patch improves performance of the Google Cr-48 chromebook's
> extremely sensitive Synaptics profile sensor touchpad by filtering out
> touch events for hovering fingers.
>
> Note: We continue to use the same hard coded threshold value used in the
> single touch case as it appears this works just as well on these
> multitouch profile sensor pads as on whatever pads it was originally
> discovered.
>
> Signed-off-by: Chung-Yih Wang <cywang@chromium.org>
Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
> drivers/input/mouse/synaptics.c | 12 +++++++++---
> 1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 8081a0a..7936a1a 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -568,17 +568,22 @@ static void synaptics_report_semi_mt_slot(struct input_dev *dev, int slot,
> }
> }
>
> +static bool finger_touched(const struct synaptics_hw_state *hw)
> +{
> + return (hw->z > 30);
> +}
> +
> static void synaptics_report_semi_mt_data(struct input_dev *dev,
> const struct synaptics_hw_state *a,
> const struct synaptics_hw_state *b,
> int num_fingers)
> {
> - if (num_fingers >= 2) {
> + if ((num_fingers >= 2) && finger_touched(a) && finger_touched(b)) {
> synaptics_report_semi_mt_slot(dev, 0, true, min(a->x, b->x),
> min(a->y, b->y));
> synaptics_report_semi_mt_slot(dev, 1, true, max(a->x, b->x),
> max(a->y, b->y));
> - } else if (num_fingers == 1) {
> + } else if ((num_fingers == 1) && finger_touched(a)) {
> synaptics_report_semi_mt_slot(dev, 0, true, a->x, a->y);
> synaptics_report_semi_mt_slot(dev, 1, false, 0, 0);
> } else {
> @@ -1040,7 +1045,8 @@ static void synaptics_process_packet(struct psmouse *psmouse)
> * BTN_TOUCH has to be first as mousedev relies on it when doing
> * absolute -> relative conversion
> */
> - if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
> + if (finger_touched(&hw))
> + input_report_key(dev, BTN_TOUCH, 1);
> if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
>
> if (num_fingers > 0) {
> --
> 1.7.7.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
@ 2012-02-22 7:41 Chung-yih Wang
2012-02-22 8:38 ` Henrik Rydberg
0 siblings, 1 reply; 13+ messages in thread
From: Chung-yih Wang @ 2012-02-22 7:41 UTC (permalink / raw)
To: Alessandro Rubini
Cc: Dmitry Torokhov, linux-input, linux-kernel, djkurtz,
Chung-yih Wang
The single touch path imposes a minimum z value (with hysterisis) before
registering BTN_TOUCH. Apply the same (hard coded) threshold when
deciding how many fingers to report in the semi-mt path.
This patch improves performance of the Google Cr-48 chromebook's
extremely sensitive Synaptics profile sensor touchpad by filtering out
touch events for hovering fingers.
Note: We continue to use the same hard coded threshold value used in the
single touch case as it appears this works just as well on these
multitouch profile sensor pads as on whatever pads it was originally
discovered.
Signed-off-by: Chung-Yih Wang <cywang@chromium.org>
---
drivers/input/mouse/synaptics.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 8081a0a..746dbcc 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -568,17 +568,22 @@ static void synaptics_report_semi_mt_slot(struct input_dev *dev, int slot,
}
}
+static bool finger_touched(const struct synaptics_hw_state *hw)
+{
+ return (hw->z > 30);
+}
+
static void synaptics_report_semi_mt_data(struct input_dev *dev,
const struct synaptics_hw_state *a,
const struct synaptics_hw_state *b,
int num_fingers)
{
- if (num_fingers >= 2) {
+ if ((num_fingers >= 2) && finger_touched(a) && finger_touched(b)) {
synaptics_report_semi_mt_slot(dev, 0, true, min(a->x, b->x),
min(a->y, b->y));
synaptics_report_semi_mt_slot(dev, 1, true, max(a->x, b->x),
max(a->y, b->y));
- } else if (num_fingers == 1) {
+ } else if ((num_fingers == 1) && finger_touched(a)) {
synaptics_report_semi_mt_slot(dev, 0, true, a->x, a->y);
synaptics_report_semi_mt_slot(dev, 1, false, 0, 0);
} else {
@@ -1040,8 +1045,10 @@ static void synaptics_process_packet(struct psmouse *psmouse)
* BTN_TOUCH has to be first as mousedev relies on it when doing
* absolute -> relative conversion
*/
- if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
- if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
+ if (finger_touched(&hw))
+ input_report_key(dev, BTN_TOUCH, 1);
+ if (hw.z < 25)
+ input_report_key(dev, BTN_TOUCH, 0);
if (num_fingers > 0) {
input_report_abs(dev, ABS_X, hw.x);
--
1.7.7.3
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-22 7:41 [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values Chung-yih Wang
@ 2012-02-22 8:38 ` Henrik Rydberg
2012-02-22 10:26 ` Daniel Kurtz
0 siblings, 1 reply; 13+ messages in thread
From: Henrik Rydberg @ 2012-02-22 8:38 UTC (permalink / raw)
To: Chung-yih Wang
Cc: Alessandro Rubini, Dmitry Torokhov, linux-input, linux-kernel,
djkurtz
Hi Chung-yih Wang,
> The single touch path imposes a minimum z value (with hysterisis) before
> registering BTN_TOUCH. Apply the same (hard coded) threshold when
> deciding how many fingers to report in the semi-mt path.
>
> This patch improves performance of the Google Cr-48 chromebook's
> extremely sensitive Synaptics profile sensor touchpad by filtering out
> touch events for hovering fingers.
>
> Note: We continue to use the same hard coded threshold value used in the
> single touch case as it appears this works just as well on these
> multitouch profile sensor pads as on whatever pads it was originally
> discovered.
>
> Signed-off-by: Chung-Yih Wang <cywang@chromium.org>
> ---
The idea is sound and has worked well in practise for a long
time. However, please see the comments inline.
> drivers/input/mouse/synaptics.c | 15 +++++++++++----
> 1 files changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 8081a0a..746dbcc 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -568,17 +568,22 @@ static void synaptics_report_semi_mt_slot(struct input_dev *dev, int slot,
> }
> }
>
> +static bool finger_touched(const struct synaptics_hw_state *hw)
> +{
> + return (hw->z > 30);
> +}
> +
> static void synaptics_report_semi_mt_data(struct input_dev *dev,
> const struct synaptics_hw_state *a,
> const struct synaptics_hw_state *b,
> int num_fingers)
> {
> - if (num_fingers >= 2) {
> + if ((num_fingers >= 2) && finger_touched(a) && finger_touched(b)) {
> synaptics_report_semi_mt_slot(dev, 0, true, min(a->x, b->x),
> min(a->y, b->y));
> synaptics_report_semi_mt_slot(dev, 1, true, max(a->x, b->x),
> max(a->y, b->y));
> - } else if (num_fingers == 1) {
> + } else if ((num_fingers == 1) && finger_touched(a)) {
> synaptics_report_semi_mt_slot(dev, 0, true, a->x, a->y);
> synaptics_report_semi_mt_slot(dev, 1, false, 0, 0);
> } else {
So if num_fingers == 2 and only one of a and b returns
finger_touched() == true, we fall back to zero fingers?
> @@ -1040,8 +1045,10 @@ static void synaptics_process_packet(struct psmouse *psmouse)
> * BTN_TOUCH has to be first as mousedev relies on it when doing
> * absolute -> relative conversion
> */
> - if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
> - if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
> + if (finger_touched(&hw))
> + input_report_key(dev, BTN_TOUCH, 1);
> + if (hw.z < 25)
> + input_report_key(dev, BTN_TOUCH, 0);
>
> if (num_fingers > 0) {
> input_report_abs(dev, ABS_X, hw.x);
Why not introduce hysteresis for all fingers here? There is an example
implementation in bcm5974.c in the same directory.
Thanks,
Henrik
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-22 8:38 ` Henrik Rydberg
@ 2012-02-22 10:26 ` Daniel Kurtz
2012-02-22 11:04 ` Henrik Rydberg
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Kurtz @ 2012-02-22 10:26 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Chung-yih Wang, Alessandro Rubini, Dmitry Torokhov, linux-input,
linux-kernel
On Wed, Feb 22, 2012 at 4:38 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Chung-yih Wang,
>
>> The single touch path imposes a minimum z value (with hysterisis) before
>> registering BTN_TOUCH. Apply the same (hard coded) threshold when
>> deciding how many fingers to report in the semi-mt path.
>>
>> This patch improves performance of the Google Cr-48 chromebook's
>> extremely sensitive Synaptics profile sensor touchpad by filtering out
>> touch events for hovering fingers.
>>
>> Note: We continue to use the same hard coded threshold value used in the
>> single touch case as it appears this works just as well on these
>> multitouch profile sensor pads as on whatever pads it was originally
>> discovered.
>>
>> Signed-off-by: Chung-Yih Wang <cywang@chromium.org>
>> ---
>
> The idea is sound and has worked well in practise for a long
> time. However, please see the comments inline.
>
>> drivers/input/mouse/synaptics.c | 15 +++++++++++----
>> 1 files changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>> index 8081a0a..746dbcc 100644
>> --- a/drivers/input/mouse/synaptics.c
>> +++ b/drivers/input/mouse/synaptics.c
>> @@ -568,17 +568,22 @@ static void synaptics_report_semi_mt_slot(struct input_dev *dev, int slot,
>> }
>> }
>>
>> +static bool finger_touched(const struct synaptics_hw_state *hw)
>> +{
>> + return (hw->z > 30);
>> +}
>> +
>> static void synaptics_report_semi_mt_data(struct input_dev *dev,
>> const struct synaptics_hw_state *a,
>> const struct synaptics_hw_state *b,
>> int num_fingers)
>> {
>> - if (num_fingers >= 2) {
>> + if ((num_fingers >= 2) && finger_touched(a) && finger_touched(b)) {
>> synaptics_report_semi_mt_slot(dev, 0, true, min(a->x, b->x),
>> min(a->y, b->y));
>> synaptics_report_semi_mt_slot(dev, 1, true, max(a->x, b->x),
>> max(a->y, b->y));
>> - } else if (num_fingers == 1) {
>> + } else if ((num_fingers == 1) && finger_touched(a)) {
>> synaptics_report_semi_mt_slot(dev, 0, true, a->x, a->y);
>> synaptics_report_semi_mt_slot(dev, 1, false, 0, 0);
>> } else {
>
> So if num_fingers == 2 and only one of a and b returns
> finger_touched() == true, we fall back to zero fingers?
Actually, yes. In this case, we will have 2 x's and 2 y's, but not
know which belong to a good finger and which belong to a too light
finger.... sigh... synaptics... sigh...
>
>> @@ -1040,8 +1045,10 @@ static void synaptics_process_packet(struct psmouse *psmouse)
>> * BTN_TOUCH has to be first as mousedev relies on it when doing
>> * absolute -> relative conversion
>> */
>> - if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
>> - if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
>> + if (finger_touched(&hw))
>> + input_report_key(dev, BTN_TOUCH, 1);
>> + if (hw.z < 25)
>> + input_report_key(dev, BTN_TOUCH, 0);
>>
>> if (num_fingers > 0) {
>> input_report_abs(dev, ABS_X, hw.x);
>
> Why not introduce hysteresis for all fingers here? There is an example
> implementation in bcm5974.c in the same directory.
Good idea, can it be in a different, follow-up patch?
>
> Thanks,
> Henrik
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-22 10:26 ` Daniel Kurtz
@ 2012-02-22 11:04 ` Henrik Rydberg
2012-02-22 13:12 ` Daniel Kurtz
0 siblings, 1 reply; 13+ messages in thread
From: Henrik Rydberg @ 2012-02-22 11:04 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Chung-yih Wang, Alessandro Rubini, Dmitry Torokhov, linux-input,
linux-kernel
Hi Daniel,
> > So if num_fingers == 2 and only one of a and b returns
> > finger_touched() == true, we fall back to zero fingers?
>
> Actually, yes. In this case, we will have 2 x's and 2 y's, but not
> know which belong to a good finger and which belong to a too light
> finger.... sigh... synaptics... sigh...
I see the problem. However, ignoring it will just move the problem
forward to another bug report, will it not? Hysteresis is a slam dunk
here. In addition, since the low-pressure state is bound to be
transitional (soon to be followed by a real num_fingers==1 package),
simply skipping such packages might be a better option.
> > Why not introduce hysteresis for all fingers here? There is an example
> > implementation in bcm5974.c in the same directory.
>
> Good idea, can it be in a different, follow-up patch?
Why should it be?
Thanks,
Henrik
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-22 11:04 ` Henrik Rydberg
@ 2012-02-22 13:12 ` Daniel Kurtz
2012-02-22 15:24 ` Henrik Rydberg
0 siblings, 1 reply; 13+ messages in thread
From: Daniel Kurtz @ 2012-02-22 13:12 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Chung-yih Wang, Alessandro Rubini, Dmitry Torokhov, linux-input,
linux-kernel
On Wed, Feb 22, 2012 at 7:04 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Daniel,
>
>> > So if num_fingers == 2 and only one of a and b returns
>> > finger_touched() == true, we fall back to zero fingers?
>>
>> Actually, yes. In this case, we will have 2 x's and 2 y's, but not
>> know which belong to a good finger and which belong to a too light
>> finger.... sigh... synaptics... sigh...
>
> I see the problem. However, ignoring it will just move the problem
> forward to another bug report, will it not? Hysteresis is a slam dunk
> here. In addition, since the low-pressure state is bound to be
> transitional (soon to be followed by a real num_fingers==1 package),
> simply skipping such packages might be a better option.
In practice, we don't actually see the profile sensor pad sending one
low-z finger, and one high-z finger. In the case where one finger is
solidly on the pad, and another finger is hovering, lifting, or
alighting, the pad sends 2 high-z fingers, with one of them having a
completely wrong x or y coordinate. The two reported z-values are
nearly, but not exactly, identical. I can't think of good fix for
this, other than adding finger tracking and filtering out via
'moved-too-far-too-fast', where possible, and I'd prefer that this be
handled in userspace. The 1-low-z && 1-high-z case that we are
discussing here isn't actually ever triggered; either both fingers are
high-z, or neither are.
The real usefulness of this patch is filtering out the 1-low-z-finger
and 2-low-z fingers cases.
As for the hypothetical 1-finger-hi, 1-finger-low case, which I asked
Chung-yih to add because it seemed like a good idea in theory...
Yes, I think you have a good point. Thanks to evdev's stateful
nature, simply skipping the (1-strong,1-weak) packet might actually
work better than forcing num_fingers == 0.
For cases where a second finger is temporarily reporting low-z because
it is arriving or leaving, evdev would just lock the (1 or 2 initial)
fingers in their current position until the transition is over, and
then start reporting the new number of fingers at their new positions.
For cases where there is one high-z finger, and a hovering thumb or
palm triggers 2-finger reporting temporarily (without ever going above
the threshold), the original finger will get frozen in its current
position until the hovering finger is no longer detected, and then
snap to its new position. This might cause strange sudden jumps, but
that seems unavoidable.
I'm not sure hysteresis is a "slam dunk"... in fact, I don't see how
it would help much. But, it is hard to argue against adding the
functionality, since the hysteresis window can be made arbitrarily
small. Perhaps if you are inclined, you can elaborate on why you
think it is important.
Thanks for you excellent feedback!
-Daniel
>
>> > Why not introduce hysteresis for all fingers here? There is an example
>> > implementation in bcm5974.c in the same directory.
>>
>> Good idea, can it be in a different, follow-up patch?
>
> Why should it be?
>
> Thanks,
> Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-22 13:12 ` Daniel Kurtz
@ 2012-02-22 15:24 ` Henrik Rydberg
2012-02-24 8:27 ` Dmitry Torokhov
0 siblings, 1 reply; 13+ messages in thread
From: Henrik Rydberg @ 2012-02-22 15:24 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Chung-yih Wang, Alessandro Rubini, Dmitry Torokhov, linux-input,
linux-kernel
> > I see the problem. However, ignoring it will just move the problem
> > forward to another bug report, will it not? Hysteresis is a slam dunk
> > here. In addition, since the low-pressure state is bound to be
> > transitional (soon to be followed by a real num_fingers==1 package),
> > simply skipping such packages might be a better option.
>
> In practice, we don't actually see the profile sensor pad sending one
> low-z finger, and one high-z finger. In the case where one finger is
> solidly on the pad, and another finger is hovering, lifting, or
> alighting, the pad sends 2 high-z fingers, with one of them having a
> completely wrong x or y coordinate.
Urgh.
> The two reported z-values are
> nearly, but not exactly, identical. I can't think of good fix for
> this, other than adding finger tracking and filtering out via
> 'moved-too-far-too-fast', where possible, and I'd prefer that this be
> handled in userspace.
It sounds like the z value in the second packet carries zero
information. If that were true, the fact that the patch is effective
suggests the semi-mt slot reporting could follow BTN_TOUCH, more or
less. In doing so, you would also obtain hysteresis automatically.
> The 1-low-z && 1-high-z case that we are
> discussing here isn't actually ever triggered; either both fingers are
> high-z, or neither are.
I suspect it depends a bit on the values of low-z and hi-z,
respectively? Otherwise, there really is no information in that extra
packet.
> The real usefulness of this patch is filtering out the 1-low-z-finger
> and 2-low-z fingers cases.
>
> As for the hypothetical 1-finger-hi, 1-finger-low case, which I asked
> Chung-yih to add because it seemed like a good idea in theory...
>
> Yes, I think you have a good point. Thanks to evdev's stateful
> nature, simply skipping the (1-strong,1-weak) packet might actually
> work better than forcing num_fingers == 0.
>
> For cases where a second finger is temporarily reporting low-z because
> it is arriving or leaving, evdev would just lock the (1 or 2 initial)
> fingers in their current position until the transition is over, and
> then start reporting the new number of fingers at their new positions.
>
> For cases where there is one high-z finger, and a hovering thumb or
> palm triggers 2-finger reporting temporarily (without ever going above
> the threshold), the original finger will get frozen in its current
> position until the hovering finger is no longer detected, and then
> snap to its new position. This might cause strange sudden jumps, but
> that seems unavoidable.
A lot of things seem unavoidable with this hardware. :-)
> I'm not sure hysteresis is a "slam dunk"... in fact, I don't see how
> it would help much. But, it is hard to argue against adding the
> functionality, since the hysteresis window can be made arbitrarily
> small. Perhaps if you are inclined, you can elaborate on why you
> think it is important.
The most striking effect is the ability to better retain a
drag. Although the statement was made in light of possible
(1-strong,1-weak) packets, it should help in the 2-weak case too.
Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-22 15:24 ` Henrik Rydberg
@ 2012-02-24 8:27 ` Dmitry Torokhov
2012-02-24 9:08 ` Henrik Rydberg
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Torokhov @ 2012-02-24 8:27 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Daniel Kurtz, Chung-yih Wang, Alessandro Rubini, linux-input,
linux-kernel
On Wed, Feb 22, 2012 at 04:24:25PM +0100, Henrik Rydberg wrote:
> > > I see the problem. However, ignoring it will just move the problem
> > > forward to another bug report, will it not? Hysteresis is a slam dunk
> > > here. In addition, since the low-pressure state is bound to be
> > > transitional (soon to be followed by a real num_fingers==1 package),
> > > simply skipping such packages might be a better option.
> >
> > In practice, we don't actually see the profile sensor pad sending one
> > low-z finger, and one high-z finger. In the case where one finger is
> > solidly on the pad, and another finger is hovering, lifting, or
> > alighting, the pad sends 2 high-z fingers, with one of them having a
> > completely wrong x or y coordinate.
>
> Urgh.
>
> > The two reported z-values are
> > nearly, but not exactly, identical. I can't think of good fix for
> > this, other than adding finger tracking and filtering out via
> > 'moved-too-far-too-fast', where possible, and I'd prefer that this be
> > handled in userspace.
>
> It sounds like the z value in the second packet carries zero
> information. If that were true, the fact that the patch is effective
> suggests the semi-mt slot reporting could follow BTN_TOUCH, more or
> less. In doing so, you would also obtain hysteresis automatically.
>
> > The 1-low-z && 1-high-z case that we are
> > discussing here isn't actually ever triggered; either both fingers are
> > high-z, or neither are.
>
> I suspect it depends a bit on the values of low-z and hi-z,
> respectively? Otherwise, there really is no information in that extra
> packet.
>
> > The real usefulness of this patch is filtering out the 1-low-z-finger
> > and 2-low-z fingers cases.
> >
> > As for the hypothetical 1-finger-hi, 1-finger-low case, which I asked
> > Chung-yih to add because it seemed like a good idea in theory...
> >
> > Yes, I think you have a good point. Thanks to evdev's stateful
> > nature, simply skipping the (1-strong,1-weak) packet might actually
> > work better than forcing num_fingers == 0.
> >
> > For cases where a second finger is temporarily reporting low-z because
> > it is arriving or leaving, evdev would just lock the (1 or 2 initial)
> > fingers in their current position until the transition is over, and
> > then start reporting the new number of fingers at their new positions.
> >
> > For cases where there is one high-z finger, and a hovering thumb or
> > palm triggers 2-finger reporting temporarily (without ever going above
> > the threshold), the original finger will get frozen in its current
> > position until the hovering finger is no longer detected, and then
> > snap to its new position. This might cause strange sudden jumps, but
> > that seems unavoidable.
>
> A lot of things seem unavoidable with this hardware. :-)
>
> > I'm not sure hysteresis is a "slam dunk"... in fact, I don't see how
> > it would help much. But, it is hard to argue against adding the
> > functionality, since the hysteresis window can be made arbitrarily
> > small. Perhaps if you are inclined, you can elaborate on why you
> > think it is important.
>
> The most striking effect is the ability to better retain a
> drag. Although the statement was made in light of possible
> (1-strong,1-weak) packets, it should help in the 2-weak case too.
The bigger question is why is this needed in kernel. The original
hysteresis with BTN_TOUCH was done for sole benefit of mousedev so that
we could allow somewhat better transition from standard PS/2 mode into
native Synaptics mode with absolute coordinates at time when barely
anyone had Synaptics X driver installed. This was, what, 10 years ago?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-24 8:27 ` Dmitry Torokhov
@ 2012-02-24 9:08 ` Henrik Rydberg
2012-02-24 9:14 ` Dmitry Torokhov
0 siblings, 1 reply; 13+ messages in thread
From: Henrik Rydberg @ 2012-02-24 9:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Daniel Kurtz, Chung-yih Wang, Alessandro Rubini, linux-input,
linux-kernel
> > The most striking effect is the ability to better retain a
> > drag. Although the statement was made in light of possible
> > (1-strong,1-weak) packets, it should help in the 2-weak case too.
>
> The bigger question is why is this needed in kernel. The original
> hysteresis with BTN_TOUCH was done for sole benefit of mousedev so that
> we could allow somewhat better transition from standard PS/2 mode into
> native Synaptics mode with absolute coordinates at time when barely
> anyone had Synaptics X driver installed. This was, what, 10 years ago?
The semi-mt behavior is obviously a special case, where userspace
relies on the reported number of fingers to transition between one
touch and two touches. There is no pressure information sent to
userspace in this case, so the situation is in fact quite similar to
the ancient mousedev situation.
Henrik
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-24 9:08 ` Henrik Rydberg
@ 2012-02-24 9:14 ` Dmitry Torokhov
2012-02-24 9:16 ` Dmitry Torokhov
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Torokhov @ 2012-02-24 9:14 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Daniel Kurtz, Chung-yih Wang, Alessandro Rubini, linux-input,
linux-kernel
On Fri, Feb 24, 2012 at 10:08:35AM +0100, Henrik Rydberg wrote:
> > > The most striking effect is the ability to better retain a
> > > drag. Although the statement was made in light of possible
> > > (1-strong,1-weak) packets, it should help in the 2-weak case too.
> >
> > The bigger question is why is this needed in kernel. The original
> > hysteresis with BTN_TOUCH was done for sole benefit of mousedev so that
> > we could allow somewhat better transition from standard PS/2 mode into
> > native Synaptics mode with absolute coordinates at time when barely
> > anyone had Synaptics X driver installed. This was, what, 10 years ago?
>
> The semi-mt behavior is obviously a special case, where userspace
> relies on the reported number of fingers to transition between one
> touch and two touches. There is no pressure information sent to
> userspace in this case, so the situation is in fact quite similar to
> the ancient mousedev situation.
We still report ABS_PRESSURE but maybe we should report ABS_MT_PRESSURE
as well?
--
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-24 9:14 ` Dmitry Torokhov
@ 2012-02-24 9:16 ` Dmitry Torokhov
2012-02-24 9:23 ` Henrik Rydberg
0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Torokhov @ 2012-02-24 9:16 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Daniel Kurtz, Chung-yih Wang, Alessandro Rubini, linux-input,
linux-kernel
On Fri, Feb 24, 2012 at 01:14:52AM -0800, Dmitry Torokhov wrote:
> On Fri, Feb 24, 2012 at 10:08:35AM +0100, Henrik Rydberg wrote:
> > > > The most striking effect is the ability to better retain a
> > > > drag. Although the statement was made in light of possible
> > > > (1-strong,1-weak) packets, it should help in the 2-weak case too.
> > >
> > > The bigger question is why is this needed in kernel. The original
> > > hysteresis with BTN_TOUCH was done for sole benefit of mousedev so that
> > > we could allow somewhat better transition from standard PS/2 mode into
> > > native Synaptics mode with absolute coordinates at time when barely
> > > anyone had Synaptics X driver installed. This was, what, 10 years ago?
> >
> > The semi-mt behavior is obviously a special case, where userspace
> > relies on the reported number of fingers to transition between one
> > touch and two touches. There is no pressure information sent to
> > userspace in this case, so the situation is in fact quite similar to
> > the ancient mousedev situation.
>
> We still report ABS_PRESSURE but maybe we should report ABS_MT_PRESSURE
> as well?
BTW, the reason I do not like this in kernel is because it dos not allow
users control touchpad sensitivity.
--
Dmitry
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values
2012-02-24 9:16 ` Dmitry Torokhov
@ 2012-02-24 9:23 ` Henrik Rydberg
0 siblings, 0 replies; 13+ messages in thread
From: Henrik Rydberg @ 2012-02-24 9:23 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Daniel Kurtz, Chung-yih Wang, Alessandro Rubini, linux-input,
linux-kernel
On Fri, Feb 24, 2012 at 01:16:02AM -0800, Dmitry Torokhov wrote:
> On Fri, Feb 24, 2012 at 01:14:52AM -0800, Dmitry Torokhov wrote:
> > On Fri, Feb 24, 2012 at 10:08:35AM +0100, Henrik Rydberg wrote:
> > > > > The most striking effect is the ability to better retain a
> > > > > drag. Although the statement was made in light of possible
> > > > > (1-strong,1-weak) packets, it should help in the 2-weak case too.
> > > >
> > > > The bigger question is why is this needed in kernel. The original
> > > > hysteresis with BTN_TOUCH was done for sole benefit of mousedev so that
> > > > we could allow somewhat better transition from standard PS/2 mode into
> > > > native Synaptics mode with absolute coordinates at time when barely
> > > > anyone had Synaptics X driver installed. This was, what, 10 years ago?
> > >
> > > The semi-mt behavior is obviously a special case, where userspace
> > > relies on the reported number of fingers to transition between one
> > > touch and two touches. There is no pressure information sent to
> > > userspace in this case, so the situation is in fact quite similar to
> > > the ancient mousedev situation.
> >
> > We still report ABS_PRESSURE but maybe we should report ABS_MT_PRESSURE
> > as well?
>
> BTW, the reason I do not like this in kernel is because it dos not allow
> users control touchpad sensitivity.
...which is perfectly reasonable, I agree. OTOH, the semi-mt devices
tend to report close-to-bogus information for anything related to
individual touches. Pressure is sadly no different, as Daniel pointed
out. I don't know what is the lesser evil here.
Henrik
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-02-24 9:23 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-22 7:41 [PATCH] CHROMIUM: Input: synaptics - filter out the events with low z values Chung-yih Wang
2012-02-22 8:38 ` Henrik Rydberg
2012-02-22 10:26 ` Daniel Kurtz
2012-02-22 11:04 ` Henrik Rydberg
2012-02-22 13:12 ` Daniel Kurtz
2012-02-22 15:24 ` Henrik Rydberg
2012-02-24 8:27 ` Dmitry Torokhov
2012-02-24 9:08 ` Henrik Rydberg
2012-02-24 9:14 ` Dmitry Torokhov
2012-02-24 9:16 ` Dmitry Torokhov
2012-02-24 9:23 ` Henrik Rydberg
-- strict thread matches above, loose matches on Subject: below --
2012-02-21 15:07 Chung-yih Wang
2012-02-21 16:35 ` Daniel Kurtz
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).