Re: [PATCH libinput] touchpad: short-circuit the edge scroll handling when it's not enabled
On Tue, May 31, 2016 at 10:18:58AM +0200, Hans de Goede wrote:
> Hi,
>
> On 31-05-16 02:16, Peter Hutterer wrote:
> > No need to handle events properly in the edge scroll state machine when it's
> > not enabled. Just set any beginning touch to state AREA and move on. The
> > rest
> > of the code guarantees neutral state when edge scrolling is enabled or
> > disabled.
> >
> > This reduces the debug output produced by libinput-debug-events when edge
> > scrolling is disabled, preventing users from seemingly identifying
> > bugs where there are none.
> >
> > Signed-off-by: Peter Hutterer
> > ---
> > src/evdev-mt-touchpad-edge-scroll.c | 12 +---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/evdev-mt-touchpad-edge-scroll.c
> > b/src/evdev-mt-touchpad-edge-scroll.c
> > index fcc0512..8222bba 100644
> > --- a/src/evdev-mt-touchpad-edge-scroll.c
> > +++ b/src/evdev-mt-touchpad-edge-scroll.c
> > @@ -318,6 +318,15 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp,
> > uint64_t time)
> > {
> > struct tp_touch *t;
> >
> > + if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE) {
> > + tp_for_each_touch(tp, t) {
> > + if (t->state == TOUCH_BEGIN)
> > + t->scroll.edge_state =
> > + EDGE_SCROLL_TOUCH_STATE_AREA;
> > + }
> > + return;
> > + }
> > +
> > tp_for_each_touch(tp, t) {
> > if (!t->dirty)
> > continue;
>
> IMHO it would be cleaner to replace this hunk with:
>
> @@ -141,7 +141,8 @@ tp_edge_scroll_handle_none(struct tp_dispatch *tp,
>
> switch (event) {
> case SCROLL_EVENT_TOUCH:
> - if (tp_touch_get_edge(tp, t)) {
> + if (tp->scroll.method == LIBINPUT_CONFIG_SCROLL_EDGE &&
> + tp_touch_get_edge(tp, t)) {
> tp_edge_scroll_set_state(tp, t,
> EDGE_SCROLL_TOUCH_STATE_EDGE_NEW);
> } else {
>
> This is the cleanest solution IMHO, but then we still get one log_debug for
> new touches;
> alternatively we could do:
unfortunately in both of your cases we still get the verbose output. this
hunk has no real effect because if edge scrolling is disabled,
tp_touch_get_edge() is always false.
>
> @@ -327,7 +327,13 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp,
> uint64_t time)
> case TOUCH_HOVERING:
> break;
> case TOUCH_BEGIN:
> - tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_TOUCH);
> + if (tp->scroll.method == LIBINPUT_CONFIG_SCROLL_EDGE) {
> + tp_edge_scroll_handle_event(tp, t,
> + SCROLL_EVENT_TOUCH);
> + } else {
> + tp_edge_scroll_set_state(tp, t,
> + EDGE_SCROLL_TOUCH_STATE_AREA);
> + }
> break;
> case TOUCH_UPDATE:
> tp_edge_scroll_handle_event(tp, t,
> SCROLL_EVENT_MOTION);
and in this case we're still calling into tp_edge_scroll_handle_event() from
TOUCH_UPDATE/END and timeouts. and that is where the log messages come from.
> That at least avoids the double tp_for_each_touch(tp, t) {}
>
> I've a slight preference for the first solution, and just living with the
> one debug line for a new touch, after all this is for debugging only, and
> code-wise it is by far the cleanest.
>
> Anyways all 3 get the job done in the end, so this patch is:
>
> Reviewed-by: Hans de Goede
thanks, I think I'll stick with the originally proposed patch. it's not as
nice but it keeps things in one place and does the job.
Cheers,
Peter
>
> Feel free to pick any of the outlined solutions (including your original one).
>
> > @@ -350,9 +359,6 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp,
> > uint64_t time)
> > const struct normalized_coords zero = { 0.0, 0.0 };
> > const struct discrete_coords zero_discrete = { 0.0, 0.0 };
> >
> > - if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE)
> > - return 0;
> > -
> > tp_for_each_touch(tp, t) {
> > if (!t->dirty)
> > continue;
> >
>
> Regards,
>
> Hans
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH libinput] touchpad: short-circuit the edge scroll handling when it's not enabled
Hi,
On 31-05-16 02:16, Peter Hutterer wrote:
No need to handle events properly in the edge scroll state machine when it's
not enabled. Just set any beginning touch to state AREA and move on. The rest
of the code guarantees neutral state when edge scrolling is enabled or
disabled.
This reduces the debug output produced by libinput-debug-events when edge
scrolling is disabled, preventing users from seemingly identifying
bugs where there are none.
Signed-off-by: Peter Hutterer
---
src/evdev-mt-touchpad-edge-scroll.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/evdev-mt-touchpad-edge-scroll.c
b/src/evdev-mt-touchpad-edge-scroll.c
index fcc0512..8222bba 100644
--- a/src/evdev-mt-touchpad-edge-scroll.c
+++ b/src/evdev-mt-touchpad-edge-scroll.c
@@ -318,6 +318,15 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp,
uint64_t time)
{
struct tp_touch *t;
+ if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE) {
+ tp_for_each_touch(tp, t) {
+ if (t->state == TOUCH_BEGIN)
+ t->scroll.edge_state =
+ EDGE_SCROLL_TOUCH_STATE_AREA;
+ }
+ return;
+ }
+
tp_for_each_touch(tp, t) {
if (!t->dirty)
continue;
IMHO it would be cleaner to replace this hunk with:
@@ -141,7 +141,8 @@ tp_edge_scroll_handle_none(struct tp_dispatch *tp,
switch (event) {
case SCROLL_EVENT_TOUCH:
- if (tp_touch_get_edge(tp, t)) {
+ if (tp->scroll.method == LIBINPUT_CONFIG_SCROLL_EDGE &&
+ tp_touch_get_edge(tp, t)) {
tp_edge_scroll_set_state(tp, t,
EDGE_SCROLL_TOUCH_STATE_EDGE_NEW);
} else {
This is the cleanest solution IMHO, but then we still get one log_debug for new
touches;
alternatively we could do:
@@ -327,7 +327,13 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp,
uint64_t time)
case TOUCH_HOVERING:
break;
case TOUCH_BEGIN:
- tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_TOUCH);
+ if (tp->scroll.method == LIBINPUT_CONFIG_SCROLL_EDGE) {
+ tp_edge_scroll_handle_event(tp, t,
+ SCROLL_EVENT_TOUCH);
+ } else {
+ tp_edge_scroll_set_state(tp, t,
+ EDGE_SCROLL_TOUCH_STATE_AREA);
+ }
break;
case TOUCH_UPDATE:
tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_MOTION);
That at least avoids the double tp_for_each_touch(tp, t) {}
I've a slight preference for the first solution, and just living with the
one debug line for a new touch, after all this is for debugging only, and
code-wise it is by far the cleanest.
Anyways all 3 get the job done in the end, so this patch is:
Reviewed-by: Hans de Goede
Feel free to pick any of the outlined solutions (including your original one).
@@ -350,9 +359,6 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t
time)
const struct normalized_coords zero = { 0.0, 0.0 };
const struct discrete_coords zero_discrete = { 0.0, 0.0 };
- if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE)
- return 0;
-
tp_for_each_touch(tp, t) {
if (!t->dirty)
continue;
Regards,
Hans
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH libinput] touchpad: short-circuit the edge scroll handling when it's not enabled
No need to handle events properly in the edge scroll state machine when it's
not enabled. Just set any beginning touch to state AREA and move on. The rest
of the code guarantees neutral state when edge scrolling is enabled or
disabled.
This reduces the debug output produced by libinput-debug-events when edge
scrolling is disabled, preventing users from seemingly identifying
bugs where there are none.
Signed-off-by: Peter Hutterer
---
src/evdev-mt-touchpad-edge-scroll.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/evdev-mt-touchpad-edge-scroll.c
b/src/evdev-mt-touchpad-edge-scroll.c
index fcc0512..8222bba 100644
--- a/src/evdev-mt-touchpad-edge-scroll.c
+++ b/src/evdev-mt-touchpad-edge-scroll.c
@@ -318,6 +318,15 @@ tp_edge_scroll_handle_state(struct tp_dispatch *tp,
uint64_t time)
{
struct tp_touch *t;
+ if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE) {
+ tp_for_each_touch(tp, t) {
+ if (t->state == TOUCH_BEGIN)
+ t->scroll.edge_state =
+ EDGE_SCROLL_TOUCH_STATE_AREA;
+ }
+ return;
+ }
+
tp_for_each_touch(tp, t) {
if (!t->dirty)
continue;
@@ -350,9 +359,6 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t
time)
const struct normalized_coords zero = { 0.0, 0.0 };
const struct discrete_coords zero_discrete = { 0.0, 0.0 };
- if (tp->scroll.method != LIBINPUT_CONFIG_SCROLL_EDGE)
- return 0;
-
tp_for_each_touch(tp, t) {
if (!t->dirty)
continue;
--
2.7.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
