input - wacom : support one finger touch the touchscreen way
Subsystems (2)
PATCH 3/4 input - wacom : support one finger touch the touchscreen way
There are two types of 1FGT devices supported in wacom_wac.c. Changing them to follow the existing touchscreen format, i.e., only report BTN_TOUCH as a valid tool type.
Touch data will be ignored if pen is in proximity. This requires a touch up sent if touch was down when pen comes in. The touch up event should be sent before any pen events emitted. Otherwise, two pointers would race for the cursor.
However, we can not send a touch up inside wacom_tpc_pen since pen and touch are on different logical port. That is why we have to check if touch is up before sending pen events.
--- drivers/input/tablet/wacom_wac.c | 125 +++++++++++++------------------------ drivers/input/tablet/wacom_wac.h | 1 + 2 files changed, 45 insertions(+), 81 deletions(-)
drivers/input/tablet/wacom_wac.c +44 -81
@@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)return 1 ; } -static void wacom_tpc_touch_out (struct wacom_wac *wacom,int idx) -{ -struct input_dev *input = wacom->input; -int finger = idx +1 ; - -input_report_abs (input,ABS_X ,0 ); -input_report_abs (input,ABS_Y ,0 ); -input_report_abs (input,ABS_MISC ,0 ); -input_report_key (input, wacom->tool[finger],0 ); -if (!idx) -input_report_key (input,BTN_TOUCH ,0 ); -input_event (input,EV_MSC ,MSC_SERIAL , finger); -input_sync (input); -} - -static void wacom_tpc_touch_in (struct wacom_wac *wacom,size_t len) +static int wacom_tpc_single_touch (struct wacom_wac *wacom,size_t len) {char *data = wacom->data;struct input_dev *input = wacom->input; +bool prox =0 ; - wacom->tool[1 ] =BTN_TOOL_DOUBLETAP ; - wacom->id[0 ] =TOUCH_DEVICE_ID ; - -if (len !=WACOM_PKGLEN_TPC1FG ) { - -switch (data[0 ]) { +if (!wacom->shared->stylus_in_proximity) { +if (len ==WACOM_PKGLEN_TPC1FG ) + prox = data[0 ] &0x01 ; +else /* with capacity */ + prox = data[1 ] &0x01 ; + }else if (wacom->shared->touch_down) + prox =0 ; +else +return 0 ; -case WACOM_REPORT_TPC1FG : -input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); -input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); -input_report_abs (input,ABS_PRESSURE ,le16_to_cpup ((__le16 *)&data[6 ])); -input_report_key (input,BTN_TOUCH ,le16_to_cpup ((__le16 *)&data[6 ])); -input_report_abs (input,ABS_MISC , wacom->id[0 ]); -input_report_key (input, wacom->tool[1 ],1 ); -input_sync (input); -break ; - } - }else { +if (len ==WACOM_PKGLEN_TPC1FG ) {input_report_abs (input,ABS_X ,get_unaligned_le16 (&data[1 ]));input_report_abs (input,ABS_Y ,get_unaligned_le16 (&data[3 ])); -input_report_key (input,BTN_TOUCH ,1 ); -input_report_abs (input,ABS_MISC , wacom->id[1 ]); -input_report_key (input, wacom->tool[1 ],1 ); -input_sync (input); + }else { +input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); +input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); } +input_report_key (input,BTN_TOUCH , prox); + +/* keep prox bit to send proper out-prox event */ + wacom->shared->touch_down = prox; + +return 1 ; }static int wacom_tpc_pen (struct wacom_wac *wacom)
@@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom) prox = data[1 ] &0x20 ; -if (!wacom->shared->stylus_in_proximity) {/* first in prox */ +if (!wacom->shared->stylus_in_proximity) {/* Going into proximity select tool */ wacom->tool[0 ] = (data[1 ] &0x0c ) ?BTN_TOOL_RUBBER :BTN_TOOL_PEN ;if (wacom->tool[0 ] ==BTN_TOOL_PEN )
@@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom) wacom->id[0 ] =ERASER_DEVICE_ID ; wacom->shared->stylus_in_proximity = true; + }else if (!wacom->shared->touch_down) { +input_report_key (input,BTN_STYLUS , data[1 ] &0x02 ); +input_report_key (input,BTN_STYLUS2 , data[1 ] &0x10 ); +input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); +input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); + pressure = ((data[7 ] &0x01 ) <<8 ) | data[6 ]; +if (pressure <0 ) + pressure = features->pressure_max + pressure +1 ; +input_report_abs (input,ABS_PRESSURE , pressure); +input_report_key (input,BTN_TOUCH , data[1 ] &0x05 ); +if (!prox) {/* out-prox */ + wacom->id[0 ] =0 ; + wacom->shared->stylus_in_proximity = false; + } +input_report_key (input, wacom->tool[0 ], prox); +return 1 ; } -input_report_key (input,BTN_STYLUS , data[1 ] &0x02 ); -input_report_key (input,BTN_STYLUS2 , data[1 ] &0x10 ); -input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); -input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); - pressure = ((data[7 ] &0x01 ) <<8 ) | data[6 ]; -if (pressure <0 ) - pressure = features->pressure_max + pressure +1 ; -input_report_abs (input,ABS_PRESSURE , pressure); -input_report_key (input,BTN_TOUCH , data[1 ] &0x05 ); -if (!prox) {/* out-prox */ - wacom->id[0 ] =0 ; - wacom->shared->stylus_in_proximity = false; - } -input_report_key (input, wacom->tool[0 ], prox); -return 1 ; +return 0 ; }static int wacom_tpc_irq (struct wacom_wac *wacom,size_t len) {char *data = wacom->data; -int prox =0 ;dbg ("wacom_tpc_irq: received report #%d" , data[0 ]); -if (len ==WACOM_PKGLEN_TPC1FG || - data[0 ] ==WACOM_REPORT_TPC1FG ) {/* single touch */ - -if (wacom->shared->stylus_in_proximity) { -if (wacom->id[1 ] &0x01 ) -wacom_tpc_touch_out (wacom,0 ); - - wacom->id[1 ] =0 ; -return 0 ; - } - -if (len ==WACOM_PKGLEN_TPC1FG ) - prox = data[0 ] &0x01 ; -else /* with capacity */ - prox = data[1 ] &0x01 ; - -if (prox) -wacom_tpc_touch_in (wacom, len); -else { -wacom_tpc_touch_out (wacom,0 ); - - wacom->id[0 ] =0 ; - } -/* keep prox bit to send proper out-prox event */ - wacom->id[1 ] = prox; - }else if (data[0 ] ==WACOM_REPORT_PENABLED ) +if (len ==WACOM_PKGLEN_TPC1FG || data[0 ] ==WACOM_REPORT_TPC1FG ) +return wacom_tpc_single_touch (wacom, len); +else if (data[0 ] ==WACOM_REPORT_PENABLED )return wacom_tpc_pen (wacom);return 0 ;
@@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,/* fall through */ case TABLETPC : +__clear_bit (ABS_MISC , input_dev->absbit); +if (features->device_type ==BTN_TOOL_DOUBLETAP || features->device_type ==BTN_TOOL_TRIPLETAP ) { input_abs_set_res(input_dev, ABS_X,
@@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,input_abs_set_res (input_dev,ABS_Y ,wacom_calculate_touch_res (features->y_max, features->y_phy)); -__set_bit (BTN_TOOL_DOUBLETAP , input_dev->keybit); }if (features->device_type !=BTN_TOOL_PEN )
drivers/input/tablet/wacom_wac.h +1 -0
@@ -88,6 +88,7 @@ struct wacom_features {struct wacom_shared {bool stylus_in_proximity; +bool touch_down; };struct wacom_wac {
Signature · 1 line
1.7.4
Hi Ping,
Quoted 82 lines · Ping Cheng
There are two types of 1FGT devices supported in
wacom_wac.c. Changing them to follow the existing touchscreen format, i.e., only reportBTN_TOUCHas a valid tool type.Touch data will be ignored if pen is in proximity. This requires a touch up sent if touch was down when pen comes in. The touch up event should be sent before any pen events emitted. Otherwise, two pointers would race for the cursor.
However, we can not send a touch up inside
wacom_tpc_pensince pen and touch are on different logical port. That is why we have to check if touch is up before sending pen events.Reviewed-by Chris Bagwell <[email protected]>Signed-off-by Ping Cheng <[email protected]>--- drivers/input/tablet/wacom_wac.c | 125 +++++++++++++------------------------ drivers/input/tablet/wacom_wac.h | 1 + 2 files changed, 45 insertions(+), 81 deletions(-)drivers/input/tablet/wacom_wac.c +12
-35@@ -675,51 +675,35 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)return 1 ; } -static void wacom_tpc_touch_out (struct wacom_wac *wacom,int idx) -{ -struct input_dev *input = wacom->input; -int finger = idx +1 ; - -input_report_abs (input,ABS_X ,0 ); -input_report_abs (input,ABS_Y ,0 ); -input_report_abs (input,ABS_MISC ,0 ); -input_report_key (input, wacom->tool[finger],0 ); -if (!idx) -input_report_key (input,BTN_TOUCH ,0 ); -input_event (input,EV_MSC ,MSC_SERIAL , finger); -input_sync (input); -} - -static void wacom_tpc_touch_in (struct wacom_wac *wacom,size_t len) +static int wacom_tpc_single_touch (struct wacom_wac *wacom,size_t len) {char *data = wacom->data;struct input_dev *input = wacom->input; +bool prox =0 ; - wacom->tool[1 ] =BTN_TOOL_DOUBLETAP ; - wacom->id[0 ] =TOUCH_DEVICE_ID ; - -if (len !=WACOM_PKGLEN_TPC1FG ) { - -switch (data[0 ]) { +if (!wacom->shared->stylus_in_proximity) { +if (len ==WACOM_PKGLEN_TPC1FG ) + prox = data[0 ] &0x01 ; +else /* with capacity */ + prox = data[1 ] &0x01 ; + }else if (wacom->shared->touch_down) + prox =0 ; +else +return 0 ; -case WACOM_REPORT_TPC1FG : -input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); -input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); -input_report_abs (input,ABS_PRESSURE ,le16_to_cpup ((__le16 *)&data[6 ])); -input_report_key (input,BTN_TOUCH ,le16_to_cpup ((__le16 *)&data[6 ])); -input_report_abs (input,ABS_MISC , wacom->id[0 ]); -input_report_key (input, wacom->tool[1 ],1 ); -input_sync (input); -break ; - } - }else { +if (len ==WACOM_PKGLEN_TPC1FG ) {input_report_abs (input,ABS_X ,get_unaligned_le16 (&data[1 ]));input_report_abs (input,ABS_Y ,get_unaligned_le16 (&data[3 ])); -input_report_key (input,BTN_TOUCH ,1 ); -input_report_abs (input,ABS_MISC , wacom->id[1 ]);
drivers/input/tablet/wacom_wac.c static int wacom_intuos_irq(struct wacom_wac *wacom) · line 704
Quoted 10 lines
drivers/input/tablet/wacom_wac.c +7
-2-input_report_key (input, wacom->tool[1 ],1 ); -input_sync (input); + }else { +input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); +input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); } +input_report_key (input,BTN_TOUCH , prox); + +/* keep prox bit to send proper out-prox event */ + wacom->shared->touch_down = prox;
So in this function, touch-first-then-pen leads to touch_down = touch_is_actually_down && !pen_in_prox. Conversely, pen-first-then-touch second leads to no state transition at all, which seems equivalent. However, the code mixes the actual pen and touch state with what gets reported. It seems cleaner to let touch_down = touch_is_actually_down, and instead define what should be reported in that case.
Quoted 24 lines
drivers/input/tablet/wacom_wac.c +9
-1+ +return 1 ; }static int wacom_tpc_pen (struct wacom_wac *wacom)@@ -731,7 +715,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom) prox = data[1 ] &0x20 ; -if (!wacom->shared->stylus_in_proximity) {/* first in prox */ +if (!wacom->shared->stylus_in_proximity) {/* Going into proximity select tool */ wacom->tool[0 ] = (data[1 ] &0x0c ) ?BTN_TOOL_RUBBER :BTN_TOOL_PEN ;if (wacom->tool[0 ] ==BTN_TOOL_PEN )@@ -740,58 +724,36 @@ static int wacom_tpc_pen(struct wacom_wac *wacom) wacom->id[0 ] =ERASER_DEVICE_ID ; wacom->shared->stylus_in_proximity = true; + }else if (!wacom->shared->touch_down) { +input_report_key (input,BTN_STYLUS , data[1 ] &0x02 ); +input_report_key (input,BTN_STYLUS2 , data[1 ] &0x10 ); +input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); +input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); + pressure = ((data[7 ] &0x01 ) <<8 ) | data[6 ];
drivers/input/tablet/wacom_wac.c static int wacom_tpc_pen(struct wacom_wac *wacom) · line 742
Quoted 10 lines
drivers/input/tablet/wacom_wac.c +10
-0+if (pressure <0 ) + pressure = features->pressure_max + pressure +1 ; +input_report_abs (input,ABS_PRESSURE , pressure); +input_report_key (input,BTN_TOUCH , data[1 ] &0x05 ); +if (!prox) {/* out-prox */ + wacom->id[0 ] =0 ; + wacom->shared->stylus_in_proximity = false; + } +input_report_key (input, wacom->tool[0 ], prox); +return 1 ;
If pen_in_prox is false on entry, and prox is false, this results in pen_in_prox being true.
Quoted 91 lines
drivers/input/tablet/wacom_wac.c +6
-43} -input_report_key (input,BTN_STYLUS , data[1 ] &0x02 ); -input_report_key (input,BTN_STYLUS2 , data[1 ] &0x10 ); -input_report_abs (input,ABS_X ,le16_to_cpup ((__le16 *)&data[2 ])); -input_report_abs (input,ABS_Y ,le16_to_cpup ((__le16 *)&data[4 ])); - pressure = ((data[7 ] &0x01 ) <<8 ) | data[6 ]; -if (pressure <0 ) - pressure = features->pressure_max + pressure +1 ; -input_report_abs (input,ABS_PRESSURE , pressure); -input_report_key (input,BTN_TOUCH , data[1 ] &0x05 ); -if (!prox) {/* out-prox */ - wacom->id[0 ] =0 ; - wacom->shared->stylus_in_proximity = false; - } -input_report_key (input, wacom->tool[0 ], prox); -return 1 ; +return 0 ; }static int wacom_tpc_irq (struct wacom_wac *wacom,size_t len) {char *data = wacom->data; -int prox =0 ;dbg ("wacom_tpc_irq: received report #%d" , data[0 ]); -if (len ==WACOM_PKGLEN_TPC1FG || - data[0 ] ==WACOM_REPORT_TPC1FG ) {/* single touch */ - -if (wacom->shared->stylus_in_proximity) { -if (wacom->id[1 ] &0x01 ) -wacom_tpc_touch_out (wacom,0 ); - - wacom->id[1 ] =0 ; -return 0 ; - } - -if (len ==WACOM_PKGLEN_TPC1FG ) - prox = data[0 ] &0x01 ; -else /* with capacity */ - prox = data[1 ] &0x01 ; - -if (prox) -wacom_tpc_touch_in (wacom, len); -else { -wacom_tpc_touch_out (wacom,0 ); - - wacom->id[0 ] =0 ; - } -/* keep prox bit to send proper out-prox event */ - wacom->id[1 ] = prox; - }else if (data[0 ] ==WACOM_REPORT_PENABLED ) +if (len ==WACOM_PKGLEN_TPC1FG || data[0 ] ==WACOM_REPORT_TPC1FG ) +return wacom_tpc_single_touch (wacom, len); +else if (data[0 ] ==WACOM_REPORT_PENABLED )return wacom_tpc_pen (wacom);return 0 ;@@ -1172,6 +1134,8 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,/* fall through */ case TABLETPC : +__clear_bit (ABS_MISC , input_dev->absbit); +if (features->device_type ==BTN_TOOL_DOUBLETAP || features->device_type ==BTN_TOOL_TRIPLETAP ) { input_abs_set_res(input_dev, ABS_X,@@ -1180,7 +1144,6 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,input_abs_set_res (input_dev,ABS_Y ,wacom_calculate_touch_res (features->y_max, features->y_phy)); -__set_bit (BTN_TOOL_DOUBLETAP , input_dev->keybit); }if (features->device_type !=BTN_TOOL_PEN )drivers/input/tablet/wacom_wac.h +1
-0@@ -88,6 +88,7 @@ struct wacom_features {struct wacom_shared {bool stylus_in_proximity; +bool touch_down; };struct wacom_wac {Signature · 1 line
1.7.4
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 4 lines
touch_downrepresents the touch status that the driver gets/sets, not the actual touch down. Driver touch down (touch_down) is decided by both the actual touch and pen status, while it won't be changed by actual touch status when pen is in.
Understood, but it hides complex state transitions and therefore becomes error prone. I believe your code is correct, but less maintainable than it could be. I would like to take the opportunity to express that here, with the following argument.
Let us assume we have two variables, A and B, representing the actual touch and pen down. We can then try to write down the boolean function for when touch and pen events should not be sent. Starting out simple, we could try, for an arbitrary time step n,
inhibit_touch(n) = T(A(n), B(n))inhibit_pen(n) = P(A(n), B(n))
Now, you already stated that this does not work very well, because if B changes state, for instance, and inhibit_touch changes state at the same time, we get a race for the pointer events. So, expanding on this, we could take history into account and try
inhibit_touch(n) = T(A(n), B(n), A(n-1), B(n-1))inhibit_pen(n) = P(A(n), B(n), A(n-1), B(n-1))
This seems like it could work, since we can detect the state change and decide how to deal with the pointer events by choosing the functions T and P appropriately.
The point of all this is that code using simple input variables (A, B) and a complex boolean function (T, P) has no hidden error states. Conversely, code using complex input variables and a simple boolean function may contain an arbitrary number of hidden error states. As time goes by, those error states may be enabled by accident during a code change. Sure, both approaches lead to correct code if performed properly, but the former is less error prone and therefore easier to maintain.
Henrik