[PATCH libinput] tablet: Add a left handed mode and tests

Peter Hutterer peter.hutterer at who-t.net
Thu Jan 22 20:20:21 PST 2015


On Tue, Jan 20, 2015 at 09:58:12PM -0500, Stephen Chandler Paul wrote:
> On the majority of Wacom tablets, the buttons are on the left side, opposite of
> the side where the palm is meant to rest. Because of this, it's impossible to
> use the tablet with your left hand (comfortably, anyway) unless you flip it
> over, in which case the coordinates need to be inverted for it to match up with
> the screen properly. This is where left handed mode comes in. When enabled, it
> reverses all the coordinates so that the tablet may be rotated, and the palm
> rest on the tablet moved over to the left side.
> 
> Signed-off-by: Stephen Chandler Paul <thatslyude at gmail.com>
> ---
> 
>                                     Changes
> * Fix formula for inverting axis
> * Separate formula for inverting axis into it's own function
> * Check when tablet_change_to_left_handed is called that the tablet tool is out
>   of proximity, otherwise don't set left_handed mode just yet
> * Style: Use litest_wait_for_event_of_type() instead of checking the event type
>   in the while loop
> * Make sure we use different values for each axis when testing, to avoid missing
>   x = y errors
> * Test against maximum and minimum values for the X and Y axis
> 
>  src/evdev-tablet.c | 29 +++++++++++++++++++++-
>  test/tablet.c      | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 101 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index f80d642..332a51a 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -86,6 +86,21 @@ tablet_mark_all_axes_changed(struct tablet_dispatch *tablet,
>  }
>  
>  static void
> +tablet_change_to_left_handed(struct evdev_device *device)
> +{
> +	struct tablet_dispatch *tablet =
> +		(struct tablet_dispatch*)device->dispatch;
> +
> +	if (device->left_handed.enabled == device->left_handed.want_enabled)
> +		return;
> +
> +	if (!tablet_has_status(tablet, TABLET_TOOL_OUT_OF_PROXIMITY))
> +		return;
> +
> +	device->left_handed.enabled = device->left_handed.want_enabled;
> +}
> +
> +static void
>  tablet_update_tool(struct tablet_dispatch *tablet,
>  		   struct evdev_device *device,
>  		   enum libinput_tool_type tool,
> @@ -120,6 +135,11 @@ normalize_tilt(const struct input_absinfo * absinfo) {
>  	return (value * 2) - 1;
>  }
>  
> +static inline int32_t
> +invert_axis(const struct input_absinfo *absinfo) {
> +	return absinfo->maximum - (absinfo->value - absinfo->minimum);
> +}
> +
>  static void
>  tablet_check_notify_axes(struct tablet_dispatch *tablet,
>  			 struct evdev_device *device,
> @@ -142,7 +162,10 @@ tablet_check_notify_axes(struct tablet_dispatch *tablet,
>  		switch (a) {
>  		case LIBINPUT_TABLET_AXIS_X:
>  		case LIBINPUT_TABLET_AXIS_Y:
> -			tablet->axes[a] = absinfo->value;
> +			if (device->left_handed.enabled)
> +				tablet->axes[a] = invert_axis(absinfo);
> +			else
> +				tablet->axes[a] = absinfo->value;
>  			break;
>  		case LIBINPUT_TABLET_AXIS_DISTANCE:
>  		case LIBINPUT_TABLET_AXIS_PRESSURE:
> @@ -486,6 +509,8 @@ tablet_flush(struct tablet_dispatch *tablet,
>  					    tablet->axes);
>  		tablet_set_status(tablet, TABLET_TOOL_OUT_OF_PROXIMITY);
>  		tablet_unset_status(tablet, TABLET_TOOL_LEAVING_PROXIMITY);
> +
> +		tablet_change_to_left_handed(device);
>  	}
>  
>  	/* Update state */
> @@ -588,5 +613,7 @@ evdev_tablet_create(struct evdev_device *device)
>  		return NULL;
>  	}
>  
> +	evdev_init_left_handed(device, tablet_change_to_left_handed);
> +
>  	return &tablet->base;
>  }
> diff --git a/test/tablet.c b/test/tablet.c
> index 367c4db..46cafcc 100644
> --- a/test/tablet.c
> +++ b/test/tablet.c
> @@ -239,6 +239,78 @@ START_TEST(motion)
>  }
>  END_TEST
>  
> +START_TEST(left_handed)
> +{
> +	struct litest_device *dev = litest_current_device();
> +	struct libinput *li = dev->libinput;
> +	struct libinput_event *event;
> +	struct libinput_event_tablet *tablet_event;
> +	double libinput_max_x, libinput_max_y;
> +	const struct input_absinfo *evdev_axis_x, *evdev_axis_y;
> +	struct axis_replacement axes[] = {
> +		{ ABS_DISTANCE, 10 },
> +		{ -1, -1 }
> +	};
> +
> +	ck_assert(libinput_device_config_left_handed_is_available(dev->libinput_device));
> +
> +	libinput_device_get_size (dev->libinput_device,
> +				  &libinput_max_x,
> +				  &libinput_max_y);
> +	evdev_axis_x = libevdev_get_abs_info(dev->evdev, ABS_X);
> +	evdev_axis_y = libevdev_get_abs_info(dev->evdev, ABS_Y);
> +
> +	/* Test that left-handed mode doesn't go into effect until the tool has
> +	 * left proximity of the tablet */
> +	litest_tablet_proximity_in(dev, 15, 5, axes);
> +	libinput_device_config_left_handed_set(dev->libinput_device, 1);
> +	litest_drain_events(li);
> +
> +	litest_event(dev, EV_ABS, ABS_X, evdev_axis_x->minimum);
> +	litest_event(dev, EV_ABS, ABS_Y, evdev_axis_y->maximum);
> +	litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +
> +	litest_wait_for_event_of_type(li, LIBINPUT_EVENT_TABLET_AXIS, -1);
> +
> +	while ((event = libinput_get_event(li))) {
> +		double x, y;
> +		tablet_event = libinput_event_get_tablet_event(event);
> +
> +		x = libinput_event_tablet_get_axis_value(
> +		    tablet_event, LIBINPUT_TABLET_AXIS_X);
> +		y = libinput_event_tablet_get_axis_value(
> +		    tablet_event, LIBINPUT_TABLET_AXIS_Y);

formatting: pls indent this further so it's immediately obvious that the
second line is a list of arguments.

> +
> +		litest_assert_double_eq(x, 0);
> +		litest_assert_double_eq(y, libinput_max_y);
> +
> +		libinput_event_destroy(event);
> +	}

you only expect one event here, so you can drop the loop (but see below).

> +
> +	/* Check that the coordinates invert once we take the tool out of
> +	 * proximity */
> +	litest_tablet_proximity_out(dev);
> +	litest_tablet_proximity_in(dev, 0, 100, axes);
> +
> +	litest_wait_for_event_of_type(li, LIBINPUT_EVENT_TABLET_AXIS, -1);
> +
> +	while ((event = libinput_get_event(li))) {
> +		double x, y;
> +		tablet_event = libinput_event_get_tablet_event(event);
> +
> +		x = libinput_event_tablet_get_axis_value(
> +		    tablet_event, LIBINPUT_TABLET_AXIS_X);
> +		y = libinput_event_tablet_get_axis_value(
> +		    tablet_event, LIBINPUT_TABLET_AXIS_Y);
> +
> +		litest_assert_double_eq(x, libinput_max_x);
> +		litest_assert_double_eq(y, 0);
> +
> +		libinput_event_destroy(event);
> +	}

hmm, looking at this shows that our behaviour is a bit odd. for the test
above you have a prox-in, which also generates an AXIS event. You drain
those, then move and test for left-handed on the second movement.

on the second test, you just drop the proximity event and check the axis
event generatd by that (no following movement). that's both good (two
different paths) and bad (inconsistent, especially because there's no
comment suggesting this is the intention).

how about:
go prox in at bottom-left, move to top right. make sure the first one
is at the matching min/max and following events are > or < the previous
coordinates.

then repeat the same for left-handed where the > and < should change place.

Semi-related: our proximity event doesn't have any data, that's in the
following axis event. I can't remember our discussions here, was this
intentional? or should we attach axis information to the proximity event so
we don't need to send the first AXIS event.

And related to that - should we merge the proximity events into a single
event and make the state a flag on the event, similar to the button/key
events that have a single type but a state on top.

Cheers,
   Peter


> +}
> +END_TEST
> +
>  START_TEST(motion_event_state)
>  {
>  	struct litest_device *dev = litest_current_device();
> @@ -869,6 +941,7 @@ main(int argc, char **argv)
>  	litest_add("tablet:proximity", bad_distance_events, LITEST_TABLET | LITEST_DISTANCE, LITEST_ANY);
>  	litest_add("tablet:motion", motion, LITEST_TABLET, LITEST_ANY);
>  	litest_add("tablet:motion", motion_event_state, LITEST_TABLET, LITEST_ANY);
> +	litest_add("tablet:left_handed", left_handed, LITEST_TABLET, LITEST_ANY);
>  	litest_add("tablet:normalization", normalization, LITEST_TABLET, LITEST_ANY);
>  	litest_add("tablet:pad", pad_buttons_ignored, LITEST_TABLET, LITEST_ANY);
>  
> -- 
> 2.0.5
> 


More information about the wayland-devel mailing list