[PATCH libinput v2 2/5] tablet: Replace tool-update with proximity-in

Peter Hutterer peter.hutterer at who-t.net
Thu Jun 26 00:44:55 PDT 2014


On Thu, Jun 26, 2014 at 02:31:19AM -0400, Stephen Chandler Paul wrote:
> A proximity-in event is something we want, especially since the current drafted
> wayland spec has a proximity-in event. Adding this also makes our events more
> consistent. And since we can just report the current tool in use with
> proximity-in events, we can get rid of the tool-update event.
> 
> Signed-off-by: Stephen Chandler Paul <thatslyude at gmail.com>
> ---
>  src/evdev-tablet.c     | 50 ++++++++++++++++++++++++++------------------------
>  src/evdev-tablet.h     |  3 ++-
>  src/libinput-private.h |  6 +++---
>  src/libinput.c         | 28 ++++++++++++++--------------
>  src/libinput.h         | 27 ++++++++++++++-------------
>  test/tablet.c          | 43 +++++++++++++++++--------------------------
>  tools/event-debug.c    | 10 +++++-----
>  tools/event-gui.c      |  2 +-
>  8 files changed, 82 insertions(+), 87 deletions(-)
> 
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 584b49e..e6595bc 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -99,6 +99,7 @@ tablet_update_tool(struct tablet_dispatch *tablet,
>  			tablet_set_status(tablet, TABLET_TOOL_UPDATED);
>  		}
>  		tablet_mark_all_axes_changed(tablet, device);
> +		tablet_set_status(tablet, TABLET_TOOL_ENTERING_PROXIMITY);
>  		tablet_unset_status(tablet, TABLET_TOOL_OUT_OF_PROXIMITY);
>  	}
>  	else
> @@ -250,8 +251,6 @@ tablet_process_misc(struct tablet_dispatch *tablet,
>  		    e->value != -1) {
>  			tablet->current_tool_serial = e->value;
>  			tablet_set_status(tablet, TABLET_TOOL_UPDATED);
> -			tablet_unset_status(tablet,
> -					    TABLET_TOOL_OUT_OF_PROXIMITY);
>  		}
>  		break;
>  	default:
> @@ -263,37 +262,35 @@ tablet_process_misc(struct tablet_dispatch *tablet,
>  	}
>  }
>  
> -static void
> -tablet_notify_tool(struct tablet_dispatch *tablet,
> -		   struct evdev_device *device,
> -		   uint32_t time)
> +static struct libinput_tool *
> +tablet_get_tool(struct libinput *li,
> +		enum libinput_tool_type type,
> +		uint32_t serial)
>  {
> -	struct libinput_device *base = &device->base;
> -	struct libinput_tool *tool;
> -	struct libinput_tool *new_tool = NULL;
> +	struct libinput_tool *tool = NULL, *t;
>  
>  	/* Check if we already have the tool in our list of tools */
> -	list_for_each(tool, &base->seat->libinput->tool_list, link) {
> -		if (tablet->current_tool_type == tool->type &&
> -		    tablet->current_tool_serial == tool->serial) {
> -			new_tool = tool;
> +	list_for_each(t, &li->tool_list, link) {
> +		if (type == t->type && serial == t->serial) {
> +			tool = t;
>  			break;
>  		}
>  	}
>  
> -	/* If we didn't already have the tool in our list of tools, add it */
> -	if (new_tool == NULL) {
> -		new_tool = zalloc(sizeof *new_tool);
> -		*new_tool = (struct libinput_tool) {
> -			.type = tablet->current_tool_type,
> -			.serial = tablet->current_tool_serial,
> +	/* If we didn't already have the new_tool in our list of tools,
> +	 * add it */
> +	if (!tool) {
> +		tool = zalloc(sizeof *tool);
> +		*tool = (struct libinput_tool) {
> +			.type = type,
> +			.serial = serial,
>  			.refcount = 1,
>  		};
>  
> -		list_insert(&base->seat->libinput->tool_list, &new_tool->link);
> +		list_insert(&li->tool_list, &tool->link);
>  	}
>  
> -	tablet_notify_tool_update(base, time, new_tool);
> +	return tool;

things like this are a lot easier to review if you first move the code into
the new function and call it from the original place, then, in a separate
commit, add whatever else is there on top. That way it's also easy to bisect
if a bug was caused by the move, or by a new feature.
please split this up in to two commits.

>  }
>  
>  static void
> @@ -379,13 +376,18 @@ tablet_flush(struct tablet_dispatch *tablet,
>  	     struct evdev_device *device,
>  	     uint32_t time)
>  {
> +	struct libinput_tool *tool =
> +		tablet_get_tool(device->base.seat->libinput,
> +				tablet->current_tool_type,
> +				tablet->current_tool_serial);
> +
>  	if (tablet_has_status(tablet, TABLET_TOOL_LEAVING_PROXIMITY)) {
>  		/* Release all stylus buttons */
>  		tablet->button_state.stylus_buttons = 0;
>  		tablet_set_status(tablet, TABLET_BUTTONS_RELEASED);
> -	} else if (tablet_has_status(tablet, TABLET_TOOL_UPDATED)) {
> -		tablet_notify_tool(tablet, device, time);
> -		tablet_unset_status(tablet, TABLET_TOOL_UPDATED);
> +	} else if (tablet_has_status(tablet, TABLET_TOOL_ENTERING_PROXIMITY)) {
> +		tablet_notify_proximity_in(&device->base, time, tool);
> +		tablet_unset_status(tablet, TABLET_TOOL_ENTERING_PROXIMITY);
>  	}
>  
>  	if (tablet_has_status(tablet, TABLET_AXES_UPDATED)) {
> diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
> index 89cf224..e6e66d7 100644
> --- a/src/evdev-tablet.h
> +++ b/src/evdev-tablet.h
> @@ -35,7 +35,8 @@ enum tablet_status {
>  	TABLET_BUTTONS_RELEASED = 1 << 3,
>  	TABLET_STYLUS_IN_CONTACT = 1 << 4,
>  	TABLET_TOOL_LEAVING_PROXIMITY = 1 << 5,
> -	TABLET_TOOL_OUT_OF_PROXIMITY = 1 << 6
> +	TABLET_TOOL_OUT_OF_PROXIMITY = 1 << 6,
> +	TABLET_TOOL_ENTERING_PROXIMITY = 1 << 7
>  };
>  
>  struct button_state {
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index db78cca..e59fe12 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -218,9 +218,9 @@ tablet_notify_axis(struct libinput_device *device,
>  		   double *axes);
>  
>  void
> -tablet_notify_tool_update(struct libinput_device *device,
> -			  uint32_t time,
> -			  struct libinput_tool *tool);
> +tablet_notify_proximity_in(struct libinput_device *device,
> +			   uint32_t time,
> +			   struct libinput_tool *tool);
>  
>  void
>  tablet_notify_proximity_out(struct libinput_device *device,
> diff --git a/src/libinput.c b/src/libinput.c
> index e72ad8b..3726eb5 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -195,7 +195,7 @@ libinput_event_get_pointer_event(struct libinput_event *event)
>  	case LIBINPUT_EVENT_TOUCH_CANCEL:
>  	case LIBINPUT_EVENT_TOUCH_FRAME:
>  	case LIBINPUT_EVENT_TABLET_AXIS:
> -	case LIBINPUT_EVENT_TABLET_TOOL_UPDATE:
> +	case LIBINPUT_EVENT_TABLET_PROXIMITY_IN:
>  	case LIBINPUT_EVENT_TABLET_PROXIMITY_OUT:
>  	case LIBINPUT_EVENT_TABLET_BUTTON:
>  		break;
> @@ -225,7 +225,7 @@ libinput_event_get_keyboard_event(struct libinput_event *event)
>  	case LIBINPUT_EVENT_TOUCH_CANCEL:
>  	case LIBINPUT_EVENT_TOUCH_FRAME:
>  	case LIBINPUT_EVENT_TABLET_AXIS:
> -	case LIBINPUT_EVENT_TABLET_TOOL_UPDATE:
> +	case LIBINPUT_EVENT_TABLET_PROXIMITY_IN:
>  	case LIBINPUT_EVENT_TABLET_PROXIMITY_OUT:
>  	case LIBINPUT_EVENT_TABLET_BUTTON:
>  		break;
> @@ -255,7 +255,7 @@ libinput_event_get_touch_event(struct libinput_event *event)
>  	case LIBINPUT_EVENT_TOUCH_FRAME:
>  		return (struct libinput_event_touch *) event;
>  	case LIBINPUT_EVENT_TABLET_AXIS:
> -	case LIBINPUT_EVENT_TABLET_TOOL_UPDATE:
> +	case LIBINPUT_EVENT_TABLET_PROXIMITY_IN:
>  	case LIBINPUT_EVENT_TABLET_PROXIMITY_OUT:
>  	case LIBINPUT_EVENT_TABLET_BUTTON:
>  		break;
> @@ -284,7 +284,7 @@ libinput_event_get_tablet_event(struct libinput_event *event)
>  	case LIBINPUT_EVENT_TOUCH_FRAME:
>  		break;
>  	case LIBINPUT_EVENT_TABLET_AXIS:
> -	case LIBINPUT_EVENT_TABLET_TOOL_UPDATE:
> +	case LIBINPUT_EVENT_TABLET_PROXIMITY_IN:
>  	case LIBINPUT_EVENT_TABLET_PROXIMITY_OUT:
>  	case LIBINPUT_EVENT_TABLET_BUTTON:
>  		return (struct libinput_event_tablet *) event;
> @@ -313,7 +313,7 @@ libinput_event_get_device_notify_event(struct libinput_event *event)
>  	case LIBINPUT_EVENT_TOUCH_CANCEL:
>  	case LIBINPUT_EVENT_TOUCH_FRAME:
>  	case LIBINPUT_EVENT_TABLET_AXIS:
> -	case LIBINPUT_EVENT_TABLET_TOOL_UPDATE:
> +	case LIBINPUT_EVENT_TABLET_PROXIMITY_IN:
>  	case LIBINPUT_EVENT_TABLET_PROXIMITY_OUT:
>  	case LIBINPUT_EVENT_TABLET_BUTTON:
>  		break;
> @@ -1250,24 +1250,24 @@ tablet_notify_axis(struct libinput_device *device,
>  }
>  
>  void
> -tablet_notify_tool_update(struct libinput_device *device,
> -			  uint32_t time,
> -			  struct libinput_tool *tool)
> +tablet_notify_proximity_in(struct libinput_device *device,
> +			   uint32_t time,
> +			   struct libinput_tool *tool)
>  {
> -	struct libinput_event_tablet *tool_update_event;
> +	struct libinput_event_tablet *proximity_in_event;
>  
> -	tool_update_event = zalloc(sizeof *tool_update_event);
> -	if (!tool_update_event)
> +	proximity_in_event = zalloc(sizeof *proximity_in_event);
> +	if (!proximity_in_event)
>  		return;
>  
> -	*tool_update_event = (struct libinput_event_tablet) {
> +	*proximity_in_event = (struct libinput_event_tablet) {
>  		.time = time,
>  		.tool = tool,
>  	};
>  
>  	post_device_event(device,
> -			  LIBINPUT_EVENT_TABLET_TOOL_UPDATE,
> -			  &tool_update_event->base);
> +			  LIBINPUT_EVENT_TABLET_PROXIMITY_IN,
> +			  &proximity_in_event->base);
>  }
>  
>  void
> diff --git a/src/libinput.h b/src/libinput.h
> index 38b6610..49162dd 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -261,10 +261,10 @@ enum libinput_event_type {
>  
>  	LIBINPUT_EVENT_TABLET_AXIS = 600,
>  	/**
> -	 * Signals that a device with the @ref LIBINPUT_DEVICE_CAP_TABLET
> -	 * capability has changed its tool.
> +	 * Signals that a tool has come into proximity of a device with the @ref
> +	 * LIBINPUT_DEVICE_CAP_TABLET capability.
>  	 */
> -	LIBINPUT_EVENT_TABLET_TOOL_UPDATE,
> +	LIBINPUT_EVENT_TABLET_PROXIMITY_IN,
>  	/**
>  	 * Signals that a device with the @ref LIBINPUT_DEVICE_CAP_TABLET
>  	 * capability has detected that there is no longer a tool in use. When
> @@ -305,8 +305,8 @@ struct libinput_event_touch;
>   *
>   * Tablet event representing an axis update, button press, or tool update. Valid
>   * event types for this event are @ref LIBINPUT_EVENT_TABLET_AXIS, @ref
> - * LIBINPUT_EVENT_TABLET_TOOL_UPDATE, @ref LIBINPUT_EVENT_TABLET_TOOL_UPDATE and
> - * @ref LIBINPUT_EVENT_TABLET_BUTTON.
> + * LIBINPUT_EVENT_TABLET_PROXIMITY_IN, @ref LIBINPUT_EVENT_TABLET_PROXIMITY_IN
> + * and @ref LIBINPUT_EVENT_TABLET_BUTTON.
>   */
>  struct libinput_event_tablet;
>  
> @@ -918,17 +918,18 @@ libinput_event_tablet_get_y_transformed(struct libinput_event_tablet *event,
>  /**
>   * @ingroup event_tablet
>   *
> - * Return the new tool in use for this event.
> + * Returns the tool that came into proximity for this event.
>   * For tablet events that are not of type @ref
> - * LIBINPUT_EVENT_TABLET_TOOL_UPDATE, this function returns NULL. By default,
> - * the lifetime of each tool is bound to the lifetime of the event, so the tool
> - * will be destroyed when the event is destroyed. However, the lifetime of the
> - * tool may be extended by using libinput_tool_ref() to increment the reference
> - * count of the tool. Whenever libinput detects that the tool is in proximity of
> - * any tablet that's connected, it will return the same libinput_tool object.
> + * LIBINPUT_EVENT_TABLET_PROXIMITY_IN, this function returns NULL. By default,
> + * the lifetime of each tool will stay valid for as long as it is being used,
> + * and is destroyed when another tool comes into proximity. However, the
> + * lifetime of the tool may be extended by using libinput_tool_ref() to
> + * increment the reference count of the tool. This guarantees that whenever the
> + * tool comes back into proximity of the tablet, that the same structure will be
> + * used to represent the tool.
>   *
>   * @note It is an application bug to call this function for events other than
> - * @ref LIBINPUT_EVENT_TABLET_TOOL_UPDATE.
> + * @ref LIBINPUT_EVENT_TABLET_PROXIMITY_IN.
>   *
>   * @note On tablets where the serial number of tools is not reported, each tool
>   * cannot be guaranteed to be unique.
> diff --git a/test/tablet.c b/test/tablet.c
> index 99200de..89da68d 100644
> --- a/test/tablet.c
> +++ b/test/tablet.c
> @@ -55,7 +55,7 @@ START_TEST(proximity_in_out)
>  
>  	while ((event = libinput_get_event(li))) {
>  		if (libinput_event_get_type(event) ==
> -		    LIBINPUT_EVENT_TABLET_TOOL_UPDATE) {
> +		    LIBINPUT_EVENT_TABLET_PROXIMITY_IN) {
>  			struct libinput_tool * tool;
>  
>  			have_tool_update++;
> @@ -427,7 +427,7 @@ START_TEST(tool_serial)
>  	libinput_dispatch(li);
>  	while ((event = libinput_get_event(li))) {
>  		if (libinput_event_get_type(event) ==
> -		    LIBINPUT_EVENT_TABLET_TOOL_UPDATE) {
> +		    LIBINPUT_EVENT_TABLET_PROXIMITY_IN) {
>  			tablet_event = libinput_event_get_tablet_event(event);
>  			tool = libinput_event_tablet_get_tool(tablet_event);
>  
> @@ -443,36 +443,35 @@ START_TEST(serial_changes_tool)
>  {
>  	struct litest_device *dev = litest_current_device();
>  	struct libinput *li = dev->libinput;
> -	struct libinput_event_tablet *tablet_event;
>  	struct libinput_event *event;
> +	struct libinput_event_tablet *tablet_event;

again, please revert that superfluous move.

>  	struct libinput_tool *tool;
> -	bool tool_updated = false;
>  
>  	litest_drain_events(li);
>  
>  	litest_event(dev, EV_KEY, BTN_TOOL_PEN, 1);
>  	litest_event(dev, EV_MSC, MSC_SERIAL, 1000);
>  	litest_event(dev, EV_SYN, SYN_REPORT, 0);
> -
> +	litest_event(dev, EV_KEY, BTN_TOOL_PEN, 0);
> +	litest_event(dev, EV_SYN, SYN_REPORT, 0);
>  	litest_drain_events(li);
>  
> +	litest_event(dev, EV_KEY, BTN_TOOL_PEN, 1);
>  	litest_event(dev, EV_MSC, MSC_SERIAL, 2000);
>  	litest_event(dev, EV_SYN, SYN_REPORT, 0);
>  
>  	libinput_dispatch(li);
>  	while ((event = libinput_get_event(li))) {
>  		if (libinput_event_get_type(event) ==
> -		    LIBINPUT_EVENT_TABLET_TOOL_UPDATE) {
> +		    LIBINPUT_EVENT_TABLET_PROXIMITY_IN) {
>  			tablet_event = libinput_event_get_tablet_event(event);
>  			tool = libinput_event_tablet_get_tool(tablet_event);
>  
>  			ck_assert_uint_eq(libinput_tool_get_serial(tool), 2000);
> -			tool_updated = true;
>  		}
>  
>  		libinput_event_destroy(event);
>  	}
> -	ck_assert(tool_updated);

again: why did you remove this check here? what happens if we get events but
none are proximity in events?

Cheers,
   Peter

>  }
>  END_TEST
>  
> @@ -481,42 +480,34 @@ START_TEST(invalid_serials)
>  	struct litest_device *dev = litest_current_device();
>  	struct libinput *li = dev->libinput;
>  	struct libinput_event *event;
> -	bool tool_updated = false;
> +	struct libinput_event_tablet *tablet_event;
> +	struct libinput_tool *tool;
>  
>  	litest_drain_events(li);
>  
>  	litest_event(dev, EV_KEY, BTN_TOOL_PEN, 1);
>  	litest_event(dev, EV_MSC, MSC_SERIAL, 1000);
>  	litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +	litest_event(dev, EV_KEY, BTN_TOOL_PEN, 0);
> +	litest_event(dev, EV_SYN, SYN_REPORT, 0);
>  	litest_drain_events(li);
>  
> +	litest_event(dev, EV_KEY, BTN_TOOL_PEN, 1);
>  	litest_event(dev, EV_MSC, MSC_SERIAL, -1);
>  	litest_event(dev, EV_SYN, SYN_REPORT, 0);
>  
>  	libinput_dispatch(li);
>  	while ((event = libinput_get_event(li))) {
>  		if (libinput_event_get_type(event) ==
> -		    LIBINPUT_EVENT_TABLET_TOOL_UPDATE)
> -			tool_updated = true;
> -
> -		libinput_event_destroy(event);
> -	}
> -	ck_assert(!tool_updated);
> -
> -	/* Make sure libinput doesn't report a tool update when the serial
> -	 * number goes back from -1 to what it was previously */
> -	litest_event(dev, EV_MSC, MSC_SERIAL, 1000);
> -	litest_event(dev, EV_SYN, SYN_REPORT, 0);
> +		    LIBINPUT_EVENT_TABLET_PROXIMITY_IN) {
> +			tablet_event = libinput_event_get_tablet_event(event);
> +			tool = libinput_event_tablet_get_tool(tablet_event);
>  
> -	libinput_dispatch(li);
> -	while ((event = libinput_get_event(li))) {
> -		if (libinput_event_get_type(event) ==
> -		    LIBINPUT_EVENT_TABLET_TOOL_UPDATE)
> -			tool_updated = true;
> +			ck_assert_uint_eq(libinput_tool_get_serial(tool), 1000);
> +		}
>  
>  		libinput_event_destroy(event);
>  	}
> -	ck_assert(!tool_updated);
>  }
>  END_TEST
>  
> diff --git a/tools/event-debug.c b/tools/event-debug.c
> index 299587b..5c6a062 100644
> --- a/tools/event-debug.c
> +++ b/tools/event-debug.c
> @@ -243,8 +243,8 @@ print_event_header(struct libinput_event *ev)
>  	case LIBINPUT_EVENT_TABLET_AXIS:
>  		type = "TABLET_AXIS";
>  		break;
> -	case LIBINPUT_EVENT_TABLET_TOOL_UPDATE:
> -		type = "TABLET_TOOL_UPDATE";
> +	case LIBINPUT_EVENT_TABLET_PROXIMITY_IN:
> +		type = "TABLET_PROXIMITY_IN";
>  		break;
>  	case LIBINPUT_EVENT_TABLET_PROXIMITY_OUT:
>  		type = "TABLET_PROXIMITY_OUT";
> @@ -424,7 +424,7 @@ print_touch_event_without_coords(struct libinput_event *ev)
>  }
>  
>  static void
> -print_tool_update_event(struct libinput_event *ev)
> +print_proximity_in_event(struct libinput_event *ev)
>  {
>  	struct libinput_event_tablet *t = libinput_event_get_tablet_event(ev);
>  	struct libinput_tool *tool = libinput_event_tablet_get_tool(t);
> @@ -543,8 +543,8 @@ handle_and_print_events(struct libinput *li)
>  		case LIBINPUT_EVENT_TABLET_AXIS:
>  			print_tablet_axis_event(ev);
>  			break;
> -		case LIBINPUT_EVENT_TABLET_TOOL_UPDATE:
> -			print_tool_update_event(ev);
> +		case LIBINPUT_EVENT_TABLET_PROXIMITY_IN:
> +			print_proximity_in_event(ev);
>  			break;
>  		case LIBINPUT_EVENT_TABLET_PROXIMITY_OUT:
>  			print_proximity_out_event(ev);
> diff --git a/tools/event-gui.c b/tools/event-gui.c
> index b184a42..7bc3828 100644
> --- a/tools/event-gui.c
> +++ b/tools/event-gui.c
> @@ -376,7 +376,7 @@ handle_event_libinput(GIOChannel *source, GIOCondition condition, gpointer data)
>  			}
>  			break;
>  		case LIBINPUT_EVENT_TABLET_AXIS:
> -		case LIBINPUT_EVENT_TABLET_TOOL_UPDATE:
> +		case LIBINPUT_EVENT_TABLET_PROXIMITY_IN:
>  		case LIBINPUT_EVENT_TABLET_PROXIMITY_OUT:
>  		case LIBINPUT_EVENT_TABLET_BUTTON:
>  			break;
> -- 
> 1.8.5.5
> 


More information about the wayland-devel mailing list