Re: touchpad slight regression (snap: 20141121-20150217)
Is anyone doing something about this regression for 5.7? The time window to apply a fix is basically over *now*. On Sat, Feb 28, 2015 at 05:01:46PM +0100, Ulf Brosziewski wrote: On 02/28/2015 09:38 AM, Martin Pieuchot wrote: [moved to tech@] On 27/02/15(Fri) 11:40, patrick keshishian wrote: Hi, On 2/26/15, Ulf Brosziewskiulf.brosziew...@t-online.de wrote: On 02/27/2015 03:31 AM, Ulf Brosziewski wrote: ... It might be that the following patch to wsmouse.c solves the problem with the new version of wsconscomm. Tests would be welcome (I could only verify that the patch does no harm to other touchpad types, i.e., Elantech-v4 and Alps Glidepoint). [...] Sorry, the change was in the wrong place and would only do a half of the work. It should look like: Index: wsmouse.c === RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v retrieving revision 1.26 diff -u -p -r1.26 wsmouse.c --- wsmouse.c 27 Oct 2014 13:55:05 - 1.26 +++ wsmouse.c 27 Feb 2015 02:50:06 - @@ -433,6 +433,9 @@ wsmouse_input(struct device *wsmousedev, } } + if (sc-sc_z == 0) + sc-sc_w = INVALID_W; + mb = sc-sc_mb; while ((d = mb ^ ub) != 0) { /* I can confirm this change alone causes no adverse, observable change on my x120e's touchpad. I the long term, I think that having a similar logic in wsmouse_input() makes sense. As I already told Ulf, it would be really nice to improve the wsmouse(4)/wscons(4) layer to support modern touchpad features and get rid of wsconscomm. But right now this chunk is really intrusive. Plus I believe it should only be applied on touchpad ``native'' mode. Sure, we could be able to check for WSMOUSE_INPUT_ABSOLUTE_W, but can it be a bad idea? What about putting a similar chunk in pms_proc_synpatics() instead? However, I would appreciate it if someone could enlighten me as to what the Z and W axis refer. I'm glad you asked, because it really depends on which hardware you're using :) Plus right now our code use magic values which are most of the time not documented. Diffs are more than welcome to improve the situation. Martin I was a bit hasty with that patch. I reasoned that sc_z is initialized with INT_MAX, and it is only used in native mode. Later I thought that change to wsconscomm might be a better (short-term) solution, something like the code below. But it doesn't look less odd than the other patch, or does it? It is not clear to me how a purely local change to pms_proc_input() could help here. As long as we don't add or redefine event types, or introduce a special convention for W values, the only way to circumvent the problem might be to produce an additional event at the start of a two-finger touch (i.e., to change W from 0 to 1 and back again). diff --git a/wsconscomm.c b/wsconscomm.c index f6b9d88..78f47ab 100644 --- a/wsconscomm.c +++ b/wsconscomm.c @@ -217,6 +217,14 @@ WSConsReadHwState(InputInfoPtr pInfo, if (hw-z == 0) { hw-fingerWidth = 0; hw-numFingers = 0; +} else if (hw-numFingers == 0) { +/* + * Because W may be 0 already, a two-finger touch on a + * Synaptics touchpad doesn't necessarily produce an update + * event for W. + */ +hw-fingerWidth = 5; +hw-numFingers = 2; } hw-millis = 1000 * event.time.tv_sec + event.time.tv_nsec / 100; SynapticsCopyHwState(hwRet, hw);
Re: touchpad slight regression (snap: 20141121-20150217)
Sorry, I must have missed this mail. This patch also seems to work with a snapshot kernel I downloaded a few minutes ago.
Re: touchpad slight regression (snap: 20141121-20150217)
[moved to tech@] On 27/02/15(Fri) 11:40, patrick keshishian wrote: Hi, On 2/26/15, Ulf Brosziewski ulf.brosziew...@t-online.de wrote: On 02/27/2015 03:31 AM, Ulf Brosziewski wrote: ... It might be that the following patch to wsmouse.c solves the problem with the new version of wsconscomm. Tests would be welcome (I could only verify that the patch does no harm to other touchpad types, i.e., Elantech-v4 and Alps Glidepoint). [...] Sorry, the change was in the wrong place and would only do a half of the work. It should look like: Index: wsmouse.c === RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v retrieving revision 1.26 diff -u -p -r1.26 wsmouse.c --- wsmouse.c 27 Oct 2014 13:55:05 - 1.26 +++ wsmouse.c 27 Feb 2015 02:50:06 - @@ -433,6 +433,9 @@ wsmouse_input(struct device *wsmousedev, } } + if (sc-sc_z == 0) + sc-sc_w = INVALID_W; + mb = sc-sc_mb; while ((d = mb ^ ub) != 0) { /* I can confirm this change alone causes no adverse, observable change on my x120e's touchpad. I the long term, I think that having a similar logic in wsmouse_input() makes sense. As I already told Ulf, it would be really nice to improve the wsmouse(4)/wscons(4) layer to support modern touchpad features and get rid of wsconscomm. But right now this chunk is really intrusive. Plus I believe it should only be applied on touchpad ``native'' mode. Sure, we could be able to check for WSMOUSE_INPUT_ABSOLUTE_W, but can it be a bad idea? What about putting a similar chunk in pms_proc_synpatics() instead? However, I would appreciate it if someone could enlighten me as to what the Z and W axis refer. I'm glad you asked, because it really depends on which hardware you're using :) Plus right now our code use magic values which are most of the time not documented. Diffs are more than welcome to improve the situation. Martin
Re: touchpad slight regression (snap: 20141121-20150217)
On 02/28/2015 09:38 AM, Martin Pieuchot wrote: [moved to tech@] On 27/02/15(Fri) 11:40, patrick keshishian wrote: Hi, On 2/26/15, Ulf Brosziewskiulf.brosziew...@t-online.de wrote: On 02/27/2015 03:31 AM, Ulf Brosziewski wrote: ... It might be that the following patch to wsmouse.c solves the problem with the new version of wsconscomm. Tests would be welcome (I could only verify that the patch does no harm to other touchpad types, i.e., Elantech-v4 and Alps Glidepoint). [...] Sorry, the change was in the wrong place and would only do a half of the work. It should look like: Index: wsmouse.c === RCS file: /cvs/src/sys/dev/wscons/wsmouse.c,v retrieving revision 1.26 diff -u -p -r1.26 wsmouse.c --- wsmouse.c 27 Oct 2014 13:55:05 - 1.26 +++ wsmouse.c 27 Feb 2015 02:50:06 - @@ -433,6 +433,9 @@ wsmouse_input(struct device *wsmousedev, } } + if (sc-sc_z == 0) + sc-sc_w = INVALID_W; + mb = sc-sc_mb; while ((d = mb ^ ub) != 0) { /* I can confirm this change alone causes no adverse, observable change on my x120e's touchpad. I the long term, I think that having a similar logic in wsmouse_input() makes sense. As I already told Ulf, it would be really nice to improve the wsmouse(4)/wscons(4) layer to support modern touchpad features and get rid of wsconscomm. But right now this chunk is really intrusive. Plus I believe it should only be applied on touchpad ``native'' mode. Sure, we could be able to check for WSMOUSE_INPUT_ABSOLUTE_W, but can it be a bad idea? What about putting a similar chunk in pms_proc_synpatics() instead? However, I would appreciate it if someone could enlighten me as to what the Z and W axis refer. I'm glad you asked, because it really depends on which hardware you're using :) Plus right now our code use magic values which are most of the time not documented. Diffs are more than welcome to improve the situation. Martin I was a bit hasty with that patch. I reasoned that sc_z is initialized with INT_MAX, and it is only used in native mode. Later I thought that change to wsconscomm might be a better (short-term) solution, something like the code below. But it doesn't look less odd than the other patch, or does it? It is not clear to me how a purely local change to pms_proc_input() could help here. As long as we don't add or redefine event types, or introduce a special convention for W values, the only way to circumvent the problem might be to produce an additional event at the start of a two-finger touch (i.e., to change W from 0 to 1 and back again). diff --git a/wsconscomm.c b/wsconscomm.c index f6b9d88..78f47ab 100644 --- a/wsconscomm.c +++ b/wsconscomm.c @@ -217,6 +217,14 @@ WSConsReadHwState(InputInfoPtr pInfo, if (hw-z == 0) { hw-fingerWidth = 0; hw-numFingers = 0; +} else if (hw-numFingers == 0) { +/* + * Because W may be 0 already, a two-finger touch on a + * Synaptics touchpad doesn't necessarily produce an update + * event for W. + */ +hw-fingerWidth = 5; +hw-numFingers = 2; } hw-millis = 1000 * event.time.tv_sec + event.time.tv_nsec / 100; SynapticsCopyHwState(hwRet, hw);