[PATCH libinput] Add libinput_event_pointer_get_axis_click_count() to count wheel clicks
Jonas Ådahl
jadahl at gmail.com
Tue Jan 13 00:11:59 PST 2015
On Tue, Jan 13, 2015 at 06:01:52PM +1000, Peter Hutterer wrote:
> On Tue, Jan 13, 2015 at 03:12:24PM +0800, Jonas Ådahl wrote:
> > On Tue, Jan 13, 2015 at 03:21:24PM +1000, Peter Hutterer wrote:
> > > The recent normalization of wheel events means we get the angle in degrees but
> > > we don't know how this corresponds to clicks. The M325 has a 20 degree click
> > > angle, most other mice have 15 degrees. So an angle of 60 can be 3 or 4 click
> > > events.
> > >
> > > Most clients care more about the click count than the angle on a mouse wheel.
> > > Provide that value when needed.
> > >
> > > Adding click_count to the axis event leaves the possibility of defining
> > > discrete units for finger/continuous scroll sources in the future. Right now,
> > > these will always reuturn 0.
> >
> > Some comments/questions inline.
> >
> > >
> > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > ---
> > > src/evdev-mt-touchpad-edge-scroll.c | 9 ++++++---
> > > src/evdev.c | 26 +++++++++++++++++---------
> > > src/libinput-private.h | 1 +
> > > src/libinput.c | 11 ++++++++++-
> > > src/libinput.h | 19 +++++++++++++++++++
> > > src/libinput.sym | 1 +
> > > test/pointer.c | 7 ++++++-
> > > 7 files changed, 60 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/src/evdev-mt-touchpad-edge-scroll.c b/src/evdev-mt-touchpad-edge-scroll.c
> > > index a4dc093..8605034 100644
> > > --- a/src/evdev-mt-touchpad-edge-scroll.c
> > > +++ b/src/evdev-mt-touchpad-edge-scroll.c
> > > @@ -327,7 +327,8 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t time)
> > > pointer_notify_axis(device, time,
> > > t->scroll.direction,
> > > LIBINPUT_POINTER_AXIS_SOURCE_FINGER,
> > > - 0.0);
> > > + 0.0,
> > > + 0);
> > > t->scroll.direction = -1;
> > > }
> > > continue;
> > > @@ -351,7 +352,8 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t time)
> > >
> > > pointer_notify_axis(device, time, axis,
> > > LIBINPUT_POINTER_AXIS_SOURCE_FINGER,
> > > - *delta);
> > > + *delta,
> > > + 0);
> > > t->scroll.direction = axis;
> > >
> > > tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_POSTED);
> > > @@ -371,7 +373,8 @@ tp_edge_scroll_stop_events(struct tp_dispatch *tp, uint64_t time)
> > > pointer_notify_axis(device, time,
> > > t->scroll.direction,
> > > LIBINPUT_POINTER_AXIS_SOURCE_FINGER,
> > > - 0.0);
> > > + 0.0,
> > > + 0);
> > > t->scroll.direction = -1;
> > > }
> > > }
> > > diff --git a/src/evdev.c b/src/evdev.c
> > > index d80594d..10ee367 100644
> > > --- a/src/evdev.c
> > > +++ b/src/evdev.c
> > > @@ -541,16 +541,20 @@ evdev_notify_axis(struct evdev_device *device,
> > > uint64_t time,
> > > enum libinput_pointer_axis axis,
> > > enum libinput_pointer_axis_source source,
> > > - double value)
> > > + double value,
> > > + double click_count)
> > > {
> > > - if (device->scroll.natural_scrolling_enabled)
> > > + if (device->scroll.natural_scrolling_enabled) {
> > > value *= -1;
> > > + click_count *= -1;
> >
> > Since clicks is a count, not a vector, it should have no direction
> > right? Meaning natural or not, a click count should always be > 0.
>
> right, that's what I thought at first too but for the 99% use-case (counting
> wheel clicks) that'd be an extra step in every client:
> click = ...get_click_count()
> value = ...get_value();
> if (value < 0)
> click = -click;
>
> handle_wheel_events(click);
>
> With the click_count using the direction, the value (and the sign of the
> value) can be ignored. I think this is preferable, and less prone to buggy
> clients.
Maybe click_value is a better term then? Or step_value? That a count is
not a count could be miss leading.
>
> [...]
>
> > > diff --git a/src/libinput.h b/src/libinput.h
> > > index f605e52..4009df3 100644
> > > --- a/src/libinput.h
> > > +++ b/src/libinput.h
> > > @@ -726,6 +726,25 @@ enum libinput_pointer_axis_source
> > > libinput_event_pointer_get_axis_source(struct libinput_event_pointer *event);
> > >
> > > /**
> > > + * @ingroup pointer
> > > + *
> > > + * Return the number of "clicks" for a given axis event. Clicks are a
> > > + * discrete scrolling unit when the value returned by
> > > + * libinput_event_pointer_get_axis_value() is not suitable.
> >
> > I assume the choice of using double to represent a click was deliberate,
> > but considering that counting usually means integer numbers, it would be
> > nice with some clarification what non-integer counts means.
>
> two reasons:
> consistency - the other pointer axis bits return doubles
> option of "subpixel clicks" in the future. Not sure how that'll work
> exactly, but if we have a click count for an otherwise continuous source, it
> could be that a distance covered is equivalent to 2.5 clicks.
> I don't have a use-case for this, and haven't thought much about how this
> would work out exactly though.
Fair enough, I guess we can leave it unsaid until we actually provide
such values.
Jonas
>
> Cheers,
> Peter
>
> > > + *
> > > + * If the source is @ref LIBINPUT_POINTER_AXIS_SOURCE_WHEEL, the clicks
> > > + * correspond to the number of physical mouse clicks.
> > > + *
> > > + * If the source is @ref LIBINPUT_POINTER_AXIS_SOURCE_CONTINUOUS or @ref
> > > + * LIBINPUT_POINTER_AXIS_SOURCE_FINGER, the click count is always 0.
> > > + *
> > > + * @return The click count for the given event. A negative number indicates
> > > + * a click count along the axis' negative direction.
> > > + */
> > > +double
> > > +libinput_event_pointer_get_axis_click_count(struct libinput_event_pointer *event);
> > > +
> > > +/**
> > > * @ingroup event_pointer
> > > *
> > > * @return The generic libinput_event of this event
> > > diff --git a/src/libinput.sym b/src/libinput.sym
> > > index 826bfde..8fccb8c 100644
> > > --- a/src/libinput.sym
> > > +++ b/src/libinput.sym
> > > @@ -71,6 +71,7 @@ global:
> > > libinput_event_pointer_get_absolute_y;
> > > libinput_event_pointer_get_absolute_y_transformed;
> > > libinput_event_pointer_get_axis;
> > > + libinput_event_pointer_get_axis_click_count;
> > > libinput_event_pointer_get_axis_source;
> > > libinput_event_pointer_get_axis_value;
> > > libinput_event_pointer_get_base_event;
> > > diff --git a/test/pointer.c b/test/pointer.c
> > > index d12c9f6..7581945 100644
> > > --- a/test/pointer.c
> > > +++ b/test/pointer.c
> > > @@ -353,9 +353,12 @@ test_wheel_event(struct litest_device *dev, int which, int amount)
> > > up by a factor 15 */
> > > const int scroll_step = 15;
> > > int expected = amount * scroll_step;
> > > + int click_count = amount;
> > >
> > > - if (libinput_device_config_scroll_get_natural_scroll_enabled(dev->libinput_device))
> > > + if (libinput_device_config_scroll_get_natural_scroll_enabled(dev->libinput_device)) {
> > > expected *= -1;
> > > + click_count *= -1;
> > > + }
> > >
> > > /* mouse scroll wheels are 'upside down' */
> > > if (which == REL_WHEEL)
> > > @@ -379,6 +382,8 @@ test_wheel_event(struct litest_device *dev, int which, int amount)
> > > ck_assert_int_eq(libinput_event_pointer_get_axis_value(ptrev), expected);
> > > ck_assert_int_eq(libinput_event_pointer_get_axis_source(ptrev),
> > > LIBINPUT_POINTER_AXIS_SOURCE_WHEEL);
> > > + ck_assert_int_eq(libinput_event_pointer_get_axis_click_count(ptrev),
> > > + click_count);
> > > libinput_event_destroy(event);
> > > }
> > >
> > > --
> > > 2.1.0
More information about the wayland-devel
mailing list