Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
Hi,
On 06/04/2014 10:57 PM, Jonas Ådahl wrote:
On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote:
Hi,
On 06/03/2014 07:34 AM, Peter Hutterer wrote:
Now that we have run-time changes of the tap.enabled state move the check
to the IDLE state only. Otherwise the tap machine may hang if tapping is
disabled while a gesture is in progress.
Two basic tests are added to check for the tap default setting - which is
now
tap disabled by default, because a bike shed's correct color is green.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad-tap.c | 56
+++--
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 32 ++
3 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index eee334f..24ea8c3 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -438,13 +438,13 @@ static void
tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t
time)
{
enum tp_tap_state current;
- if (!tp-tap.enabled)
- return;
current = tp-tap.state;
switch(tp-tap.state) {
case TAP_STATE_IDLE:
+ if (!tp-tap.enabled)
+ break;
tp_tap_idle_handle_event(tp, event, time);
break;
case TAP_STATE_TOUCH:
@@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
unsigned int
tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
{
- if (!tp-tap.enabled)
- return 0;
-
if (tp-tap.timeout tp-tap.timeout = time) {
tp_tap_clear_timer(tp);
tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
@@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t
time)
return tp-tap.timeout;
}
+static int
+tp_tap_config_count(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return min(tp-ntouches, 3); /* we only do up to 3 finger tap */
+}
+
+static enum libinput_config_status
+tp_tap_config_enable(struct libinput_device *device, int enabled)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
Please user container_of here for proper type checking rather then
just a blatant cast of $random_type_foo to $random_type_bar. Same for all
the other occurrences of the same pattern below.
Would just like to point out that there are several places this cast is
already in place, and starting to use container_of would introduce
inconsistencies. I think this kind of casting isn't necessarily bad for
single (or primary) inheritance types of objects.
I really believe that we should make use of compiler type checking were
possible. Lets go with Peters suggestion to do a separate patch to move
all these kind of casts over to container_of in one big patch.
Regards,
Hans
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Wed, Jun 04, 2014 at 08:00:17AM +1000, Peter Hutterer wrote:
On Tue, Jun 03, 2014 at 10:42:50PM +0200, Jonas Ådahl wrote:
On Tue, Jun 03, 2014 at 03:34:56PM +1000, Peter Hutterer wrote:
Now that we have run-time changes of the tap.enabled state move the check
to the IDLE state only. Otherwise the tap machine may hang if tapping is
disabled while a gesture is in progress.
Two basic tests are added to check for the tap default setting - which is
now
tap disabled by default, because a bike shed's correct color is green.
Say what? I might have missed some joke reference, but why would you
want to disable tapping by default?
dates back a couple of years where we had some amusing back/forth in the
synaptics driver in regards to tapping enabled or disabled by default.
long story short, we ended up disabling it by default for two reasons:
* if you don't know that tapping is a thing (or enabled by default), you get
spurious mouse events that make the desktop feel buggy.
* if you do know what tapping is and you want it, you usually know where to
enable it, or at least you can search for it.
Of course, there is merry disagreement on this but I still think those two
reasons above justify disabling tapping by default.
Fair enough. I won't start such a thread again, even though it means one
of my touchpads will not work by default (as the physical buttons are
broken, although reported existing :P).
I suppose however that certain models are designed to be used for
tapping having it enabled in the preinstalled system, but I wouldn't
know how to get that information.
Jonas
Cheers,
Peter
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad-tap.c | 56
+++--
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 32 ++
3 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index eee334f..24ea8c3 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -438,13 +438,13 @@ static void
tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event,
uint64_t time)
{
enum tp_tap_state current;
- if (!tp-tap.enabled)
- return;
current = tp-tap.state;
switch(tp-tap.state) {
case TAP_STATE_IDLE:
+ if (!tp-tap.enabled)
+ break;
tp_tap_idle_handle_event(tp, event, time);
break;
case TAP_STATE_TOUCH:
@@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
unsigned int
tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
{
- if (!tp-tap.enabled)
- return 0;
-
if (tp-tap.timeout tp-tap.timeout = time) {
tp_tap_clear_timer(tp);
tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
@@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp,
uint64_t time)
return tp-tap.timeout;
}
+static int
+tp_tap_config_count(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return min(tp-ntouches, 3); /* we only do up to 3 finger tap */
+}
+
+static enum libinput_config_status
+tp_tap_config_enable(struct libinput_device *device, int enabled)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
nit: (struct bla *) var, and 80 chars line limit (here and below, and in
patch 5).
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ if (tp_tap_config_count(device) == 0)
+ return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
+
+ tp-tap.enabled = enabled;
+
+ return LIBINPUT_CONFIG_STATUS_SUCCESS;
+}
+
+static int
+tp_tap_config_is_enabled(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return tp-tap.enabled;
+}
+
+static void
+tp_tap_config_reset(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ tp-tap.enabled = false;
+}
+
int
tp_init_tap(struct tp_dispatch *tp)
{
+ tp-tap.config.count = tp_tap_config_count;
+ tp-tap.config.enable = tp_tap_config_enable;
+ tp-tap.config.is_enabled = tp_tap_config_is_enabled;
+ tp-tap.config.reset = tp_tap_config_reset;
+ tp-device-base.config.tap = tp-tap.config;
+
tp-tap.state = TAP_STATE_IDLE;
tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
@@ -603,8 +647,6 @@ tp_init_tap(struct
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
Hi,
On 06/03/2014 07:34 AM, Peter Hutterer wrote:
Now that we have run-time changes of the tap.enabled state move the check
to the IDLE state only. Otherwise the tap machine may hang if tapping is
disabled while a gesture is in progress.
Two basic tests are added to check for the tap default setting - which is now
tap disabled by default, because a bike shed's correct color is green.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad-tap.c | 56
+++--
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 32 ++
3 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index eee334f..24ea8c3 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -438,13 +438,13 @@ static void
tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t
time)
{
enum tp_tap_state current;
- if (!tp-tap.enabled)
- return;
current = tp-tap.state;
switch(tp-tap.state) {
case TAP_STATE_IDLE:
+ if (!tp-tap.enabled)
+ break;
tp_tap_idle_handle_event(tp, event, time);
break;
case TAP_STATE_TOUCH:
@@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
unsigned int
tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
{
- if (!tp-tap.enabled)
- return 0;
-
if (tp-tap.timeout tp-tap.timeout = time) {
tp_tap_clear_timer(tp);
tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
@@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t
time)
return tp-tap.timeout;
}
+static int
+tp_tap_config_count(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return min(tp-ntouches, 3); /* we only do up to 3 finger tap */
+}
+
+static enum libinput_config_status
+tp_tap_config_enable(struct libinput_device *device, int enabled)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
Please user container_of here for proper type checking rather then
just a blatant cast of $random_type_foo to $random_type_bar. Same for all
the other occurrences of the same pattern below.
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ if (tp_tap_config_count(device) == 0)
+ return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
This is a repeat of the test done by the higher level wrapper, as such
this seems unnecessary by me.
Regards,
Hans
+
+ tp-tap.enabled = enabled;
+
+ return LIBINPUT_CONFIG_STATUS_SUCCESS;
+}
+
+static int
+tp_tap_config_is_enabled(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return tp-tap.enabled;
+}
+
+static void
+tp_tap_config_reset(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ tp-tap.enabled = false;
+}
+
int
tp_init_tap(struct tp_dispatch *tp)
{
+ tp-tap.config.count = tp_tap_config_count;
+ tp-tap.config.enable = tp_tap_config_enable;
+ tp-tap.config.is_enabled = tp_tap_config_is_enabled;
+ tp-tap.config.reset = tp_tap_config_reset;
+ tp-device-base.config.tap = tp-tap.config;
+
tp-tap.state = TAP_STATE_IDLE;
tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
@@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp)
return -1;
}
- tp-tap.enabled = 1; /* FIXME */
-
return 0;
}
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index d514ed6..0b31e9a 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -203,6 +203,7 @@ struct tp_dispatch {
struct {
bool enabled;
+ struct libinput_device_config_tap config;
int timer_fd;
struct libinput_source *source;
unsigned int timeout;
diff --git a/test/touchpad.c b/test/touchpad.c
index 35781c3..060b529 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap)
struct libinput *li = dev-libinput;
struct libinput_event *event;
+ libinput_device_config_tap_enable(dev-libinput_device, 1);
+
litest_drain_events(li);
litest_touch_down(dev, 0, 50, 50);
@@ -141,6 +143,8 @@ START_TEST(touchpad_1fg_tap_n_drag)
struct libinput *li = dev-libinput;
struct libinput_event *event;
+
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote:
Hi,
On 06/03/2014 07:34 AM, Peter Hutterer wrote:
Now that we have run-time changes of the tap.enabled state move the check
to the IDLE state only. Otherwise the tap machine may hang if tapping is
disabled while a gesture is in progress.
Two basic tests are added to check for the tap default setting - which is
now
tap disabled by default, because a bike shed's correct color is green.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad-tap.c | 56
+++--
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 32 ++
3 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index eee334f..24ea8c3 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -438,13 +438,13 @@ static void
tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t
time)
{
enum tp_tap_state current;
- if (!tp-tap.enabled)
- return;
current = tp-tap.state;
switch(tp-tap.state) {
case TAP_STATE_IDLE:
+ if (!tp-tap.enabled)
+ break;
tp_tap_idle_handle_event(tp, event, time);
break;
case TAP_STATE_TOUCH:
@@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
unsigned int
tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
{
- if (!tp-tap.enabled)
- return 0;
-
if (tp-tap.timeout tp-tap.timeout = time) {
tp_tap_clear_timer(tp);
tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
@@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t
time)
return tp-tap.timeout;
}
+static int
+tp_tap_config_count(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return min(tp-ntouches, 3); /* we only do up to 3 finger tap */
+}
+
+static enum libinput_config_status
+tp_tap_config_enable(struct libinput_device *device, int enabled)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
Please user container_of here for proper type checking rather then
just a blatant cast of $random_type_foo to $random_type_bar. Same for all
the other occurrences of the same pattern below.
Would just like to point out that there are several places this cast is
already in place, and starting to use container_of would introduce
inconsistencies. I think this kind of casting isn't necessarily bad for
single (or primary) inheritance types of objects.
Jonas
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ if (tp_tap_config_count(device) == 0)
+ return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
This is a repeat of the test done by the higher level wrapper, as such
this seems unnecessary by me.
Regards,
Hans
+
+ tp-tap.enabled = enabled;
+
+ return LIBINPUT_CONFIG_STATUS_SUCCESS;
+}
+
+static int
+tp_tap_config_is_enabled(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return tp-tap.enabled;
+}
+
+static void
+tp_tap_config_reset(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ tp-tap.enabled = false;
+}
+
int
tp_init_tap(struct tp_dispatch *tp)
{
+ tp-tap.config.count = tp_tap_config_count;
+ tp-tap.config.enable = tp_tap_config_enable;
+ tp-tap.config.is_enabled = tp_tap_config_is_enabled;
+ tp-tap.config.reset = tp_tap_config_reset;
+ tp-device-base.config.tap = tp-tap.config;
+
tp-tap.state = TAP_STATE_IDLE;
tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
@@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp)
return -1;
}
- tp-tap.enabled = 1; /* FIXME */
-
return 0;
}
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index d514ed6..0b31e9a 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -203,6 +203,7 @@ struct tp_dispatch {
struct {
bool enabled;
+ struct libinput_device_config_tap config;
int timer_fd;
struct libinput_source *source;
unsigned int timeout;
diff --git a/test/touchpad.c b/test/touchpad.c
index 35781c3..060b529 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap)
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Wed, Jun 04, 2014 at 10:57:36PM +0200, Jonas Ådahl wrote:
On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote:
On 06/03/2014 07:34 AM, Peter Hutterer wrote:
[...]
@@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp,
uint64_t time)
return tp-tap.timeout;
}
+static int
+tp_tap_config_count(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return min(tp-ntouches, 3); /* we only do up to 3 finger tap */
+}
+
+static enum libinput_config_status
+tp_tap_config_enable(struct libinput_device *device, int enabled)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
Please user container_of here for proper type checking rather then
just a blatant cast of $random_type_foo to $random_type_bar. Same for all
the other occurrences of the same pattern below.
Would just like to point out that there are several places this cast is
already in place, and starting to use container_of would introduce
inconsistencies. I think this kind of casting isn't necessarily bad for
single (or primary) inheritance types of objects.
I don't mind switching this to container_of, but I won't do this for this
patch. I'll switch the whole of libinput over in one go instead.
Cheers,
Peter
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Tue, Jun 03, 2014 at 03:34:56PM +1000, Peter Hutterer wrote:
Now that we have run-time changes of the tap.enabled state move the check
to the IDLE state only. Otherwise the tap machine may hang if tapping is
disabled while a gesture is in progress.
Two basic tests are added to check for the tap default setting - which is now
tap disabled by default, because a bike shed's correct color is green.
Say what? I might have missed some joke reference, but why would you
want to disable tapping by default?
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad-tap.c | 56
+++--
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 32 ++
3 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index eee334f..24ea8c3 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -438,13 +438,13 @@ static void
tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t
time)
{
enum tp_tap_state current;
- if (!tp-tap.enabled)
- return;
current = tp-tap.state;
switch(tp-tap.state) {
case TAP_STATE_IDLE:
+ if (!tp-tap.enabled)
+ break;
tp_tap_idle_handle_event(tp, event, time);
break;
case TAP_STATE_TOUCH:
@@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
unsigned int
tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
{
- if (!tp-tap.enabled)
- return 0;
-
if (tp-tap.timeout tp-tap.timeout = time) {
tp_tap_clear_timer(tp);
tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
@@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t
time)
return tp-tap.timeout;
}
+static int
+tp_tap_config_count(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return min(tp-ntouches, 3); /* we only do up to 3 finger tap */
+}
+
+static enum libinput_config_status
+tp_tap_config_enable(struct libinput_device *device, int enabled)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
nit: (struct bla *) var, and 80 chars line limit (here and below, and in
patch 5).
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ if (tp_tap_config_count(device) == 0)
+ return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
+
+ tp-tap.enabled = enabled;
+
+ return LIBINPUT_CONFIG_STATUS_SUCCESS;
+}
+
+static int
+tp_tap_config_is_enabled(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return tp-tap.enabled;
+}
+
+static void
+tp_tap_config_reset(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ tp-tap.enabled = false;
+}
+
int
tp_init_tap(struct tp_dispatch *tp)
{
+ tp-tap.config.count = tp_tap_config_count;
+ tp-tap.config.enable = tp_tap_config_enable;
+ tp-tap.config.is_enabled = tp_tap_config_is_enabled;
+ tp-tap.config.reset = tp_tap_config_reset;
+ tp-device-base.config.tap = tp-tap.config;
+
tp-tap.state = TAP_STATE_IDLE;
tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
@@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp)
return -1;
}
- tp-tap.enabled = 1; /* FIXME */
-
return 0;
}
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index d514ed6..0b31e9a 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -203,6 +203,7 @@ struct tp_dispatch {
struct {
bool enabled;
+ struct libinput_device_config_tap config;
int timer_fd;
struct libinput_source *source;
unsigned int timeout;
diff --git a/test/touchpad.c b/test/touchpad.c
index 35781c3..060b529 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap)
struct libinput *li = dev-libinput;
struct libinput_event *event;
+ libinput_device_config_tap_enable(dev-libinput_device, 1);
+
litest_drain_events(li);
litest_touch_down(dev, 0, 50, 50);
@@ -141,6 +143,8 @@ START_TEST(touchpad_1fg_tap_n_drag)
struct libinput *li = dev-libinput;
struct libinput_event *event;
+ libinput_device_config_tap_enable(dev-libinput_device, 1);
+
litest_drain_events(li);
litest_touch_down(dev, 0,
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Tue, Jun 3, 2014 at 4:42 PM, Jonas Ådahl jad...@gmail.com wrote: On Tue, Jun 03, 2014 at 03:34:56PM +1000, Peter Hutterer wrote: Now that we have run-time changes of the tap.enabled state move the check to the IDLE state only. Otherwise the tap machine may hang if tapping is disabled while a gesture is in progress. Two basic tests are added to check for the tap default setting - which is now tap disabled by default, because a bike shed's correct color is green. Say what? I might have missed some joke reference, but why would you want to disable tapping by default? Because it is the only behavior that makes sense. http://bikeshed.com/ -- Jasper ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput 03/10] touchpad: hook up to the tapping configuration
On Tue, Jun 03, 2014 at 10:42:50PM +0200, Jonas Ådahl wrote:
On Tue, Jun 03, 2014 at 03:34:56PM +1000, Peter Hutterer wrote:
Now that we have run-time changes of the tap.enabled state move the check
to the IDLE state only. Otherwise the tap machine may hang if tapping is
disabled while a gesture is in progress.
Two basic tests are added to check for the tap default setting - which is
now
tap disabled by default, because a bike shed's correct color is green.
Say what? I might have missed some joke reference, but why would you
want to disable tapping by default?
dates back a couple of years where we had some amusing back/forth in the
synaptics driver in regards to tapping enabled or disabled by default.
long story short, we ended up disabling it by default for two reasons:
* if you don't know that tapping is a thing (or enabled by default), you get
spurious mouse events that make the desktop feel buggy.
* if you do know what tapping is and you want it, you usually know where to
enable it, or at least you can search for it.
Of course, there is merry disagreement on this but I still think those two
reasons above justify disabling tapping by default.
Cheers,
Peter
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad-tap.c | 56
+++--
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 32 ++
3 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index eee334f..24ea8c3 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -438,13 +438,13 @@ static void
tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t
time)
{
enum tp_tap_state current;
- if (!tp-tap.enabled)
- return;
current = tp-tap.state;
switch(tp-tap.state) {
case TAP_STATE_IDLE:
+ if (!tp-tap.enabled)
+ break;
tp_tap_idle_handle_event(tp, event, time);
break;
case TAP_STATE_TOUCH:
@@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
unsigned int
tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
{
- if (!tp-tap.enabled)
- return 0;
-
if (tp-tap.timeout tp-tap.timeout = time) {
tp_tap_clear_timer(tp);
tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
@@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t
time)
return tp-tap.timeout;
}
+static int
+tp_tap_config_count(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return min(tp-ntouches, 3); /* we only do up to 3 finger tap */
+}
+
+static enum libinput_config_status
+tp_tap_config_enable(struct libinput_device *device, int enabled)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
nit: (struct bla *) var, and 80 chars line limit (here and below, and in
patch 5).
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ if (tp_tap_config_count(device) == 0)
+ return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
+
+ tp-tap.enabled = enabled;
+
+ return LIBINPUT_CONFIG_STATUS_SUCCESS;
+}
+
+static int
+tp_tap_config_is_enabled(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return tp-tap.enabled;
+}
+
+static void
+tp_tap_config_reset(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ tp-tap.enabled = false;
+}
+
int
tp_init_tap(struct tp_dispatch *tp)
{
+ tp-tap.config.count = tp_tap_config_count;
+ tp-tap.config.enable = tp_tap_config_enable;
+ tp-tap.config.is_enabled = tp_tap_config_is_enabled;
+ tp-tap.config.reset = tp_tap_config_reset;
+ tp-device-base.config.tap = tp-tap.config;
+
tp-tap.state = TAP_STATE_IDLE;
tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
@@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp)
return -1;
}
- tp-tap.enabled = 1; /* FIXME */
-
return 0;
}
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index d514ed6..0b31e9a 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -203,6 +203,7 @@ struct tp_dispatch {
struct {
bool enabled;
+ struct libinput_device_config_tap config;
int timer_fd;
struct libinput_source
[PATCH libinput 03/10] touchpad: hook up to the tapping configuration
Now that we have run-time changes of the tap.enabled state move the check
to the IDLE state only. Otherwise the tap machine may hang if tapping is
disabled while a gesture is in progress.
Two basic tests are added to check for the tap default setting - which is now
tap disabled by default, because a bike shed's correct color is green.
Signed-off-by: Peter Hutterer peter.hutte...@who-t.net
---
src/evdev-mt-touchpad-tap.c | 56 +++--
src/evdev-mt-touchpad.h | 1 +
test/touchpad.c | 32 ++
3 files changed, 82 insertions(+), 7 deletions(-)
diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
index eee334f..24ea8c3 100644
--- a/src/evdev-mt-touchpad-tap.c
+++ b/src/evdev-mt-touchpad-tap.c
@@ -438,13 +438,13 @@ static void
tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t
time)
{
enum tp_tap_state current;
- if (!tp-tap.enabled)
- return;
current = tp-tap.state;
switch(tp-tap.state) {
case TAP_STATE_IDLE:
+ if (!tp-tap.enabled)
+ break;
tp_tap_idle_handle_event(tp, event, time);
break;
case TAP_STATE_TOUCH:
@@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
unsigned int
tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
{
- if (!tp-tap.enabled)
- return 0;
-
if (tp-tap.timeout tp-tap.timeout = time) {
tp_tap_clear_timer(tp);
tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
@@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t
time)
return tp-tap.timeout;
}
+static int
+tp_tap_config_count(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return min(tp-ntouches, 3); /* we only do up to 3 finger tap */
+}
+
+static enum libinput_config_status
+tp_tap_config_enable(struct libinput_device *device, int enabled)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ if (tp_tap_config_count(device) == 0)
+ return LIBINPUT_CONFIG_STATUS_UNSUPPORTED;
+
+ tp-tap.enabled = enabled;
+
+ return LIBINPUT_CONFIG_STATUS_SUCCESS;
+}
+
+static int
+tp_tap_config_is_enabled(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ return tp-tap.enabled;
+}
+
+static void
+tp_tap_config_reset(struct libinput_device *device)
+{
+ struct evdev_dispatch *dispatch = ((struct evdev_device
*)device)-dispatch;
+ struct tp_dispatch *tp = container_of(dispatch, tp, base);
+
+ tp-tap.enabled = false;
+}
+
int
tp_init_tap(struct tp_dispatch *tp)
{
+ tp-tap.config.count = tp_tap_config_count;
+ tp-tap.config.enable = tp_tap_config_enable;
+ tp-tap.config.is_enabled = tp_tap_config_is_enabled;
+ tp-tap.config.reset = tp_tap_config_reset;
+ tp-device-base.config.tap = tp-tap.config;
+
tp-tap.state = TAP_STATE_IDLE;
tp-tap.timer_fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
@@ -603,8 +647,6 @@ tp_init_tap(struct tp_dispatch *tp)
return -1;
}
- tp-tap.enabled = 1; /* FIXME */
-
return 0;
}
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index d514ed6..0b31e9a 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -203,6 +203,7 @@ struct tp_dispatch {
struct {
bool enabled;
+ struct libinput_device_config_tap config;
int timer_fd;
struct libinput_source *source;
unsigned int timeout;
diff --git a/test/touchpad.c b/test/touchpad.c
index 35781c3..060b529 100644
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -116,6 +116,8 @@ START_TEST(touchpad_1fg_tap)
struct libinput *li = dev-libinput;
struct libinput_event *event;
+ libinput_device_config_tap_enable(dev-libinput_device, 1);
+
litest_drain_events(li);
litest_touch_down(dev, 0, 50, 50);
@@ -141,6 +143,8 @@ START_TEST(touchpad_1fg_tap_n_drag)
struct libinput *li = dev-libinput;
struct libinput_event *event;
+ libinput_device_config_tap_enable(dev-libinput_device, 1);
+
litest_drain_events(li);
litest_touch_down(dev, 0, 50, 50);
@@ -194,6 +198,8 @@ START_TEST(touchpad_2fg_tap)
struct libinput *li = dev-libinput;
struct libinput_event *event;
+ libinput_device_config_tap_enable(dev-libinput_device, 1);
+
litest_drain_events(dev-libinput);
litest_touch_down(dev, 0,
