[PATCH libinput] Emit LIBINPUT_TABLET_EVENT_TOOL_UPDATE events on tool changes

Peter Hutterer peter.hutterer at who-t.net
Sun Jun 15 20:54:19 PDT 2014


On Fri, Jun 13, 2014 at 04:23:19AM -0400, Stephen Chandler Paul wrote:
> V2 of this patch. There was a big change I was supposed to apply to this before
> I sent it out that completely slipped my mind until just now. This causes a
> merge conflict with the patch for handling tablet buttons so I'll be sending out
> a modified version of that in just a second
> 
> Signed-off-by: Stephen Chandler Paul <thatslyude at gmail.com>
> ---

for the innocent observer: 
the previous version had the tool list as a library-wide variable. the tool
list now is privy to the libinput context, so we can have a tool move across
devices but not across contexts.

>  src/evdev-tablet.c     | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/evdev-tablet.h     |  6 +++-
>  src/libinput-private.h |  7 ++++
>  src/libinput.c         | 41 ++++++++++++++++++++++
>  src/libinput.h         | 35 +++++++++++++++++--
>  5 files changed, 181 insertions(+), 3 deletions(-)
> 
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index 5c73bcb..c5160b1 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -63,6 +63,19 @@ tablet_process_absolute(struct tablet_dispatch *tablet,
>  }
>  
>  static void
> +tablet_update_tool(struct tablet_dispatch *tablet,
> +		   int32_t tool,

change this to the tool enum please

> +		   bool enabled)
> +{
> +	assert(tool != LIBINPUT_TOOL_NONE);
> +
> +	if (enabled && tool != tablet->current_tool_type) {
> +		tablet->current_tool_type = tool;
> +		tablet_set_status(tablet, TABLET_TOOL_UPDATED);
> +	}
> +}
> +
> +static void
>  tablet_notify_axes(struct tablet_dispatch *tablet,
>  		   struct evdev_device *device,
>  		   uint32_t time)
> @@ -95,10 +108,85 @@ tablet_notify_axes(struct tablet_dispatch *tablet,
>  }
>  
>  static void
> +tablet_process_key(struct tablet_dispatch *tablet,
> +		   struct evdev_device *device,
> +		   struct input_event *e,
> +		   uint32_t time)
> +{
> +	switch (e->code) {
> +	case BTN_TOOL_PEN:
> +	case BTN_TOOL_RUBBER:
> +	case BTN_TOOL_BRUSH:
> +	case BTN_TOOL_PENCIL:
> +	case BTN_TOOL_AIRBRUSH:
> +	case BTN_TOOL_FINGER:
> +	case BTN_TOOL_MOUSE:
> +	case BTN_TOOL_LENS:
> +		/* These codes have an equivalent libinput_tool value */
> +		tablet_update_tool(tablet, e->code, e->value);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
> +static void
> +tablet_process_misc(struct tablet_dispatch *tablet,
> +		    struct evdev_device *device,
> +		    struct input_event *e,
> +		    uint32_t time)
> +{
> +	switch (e->code) {
> +	case MSC_SERIAL:
> +		tablet->current_tool_serial = e->value;
> +		break;
> +	default:
> +		log_info("Unhandled MSC event code %#x\n", e->code);
> +		break;
> +	}
> +}
> +
> +static void
> +tablet_notify_tool(struct tablet_dispatch *tablet,
> +		   struct evdev_device *device,
> +		   uint32_t time)
> +{
> +	struct libinput_device *base = &device->base;
> +	struct libinput_tool *tool;
> +	struct libinput_tool *new_tool = NULL;
> +
> +	/* 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;
> +			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,
> +			.refcount = 1,
> +		};
> +
> +		list_insert(&base->seat->libinput->tool_list, &new_tool->link);
> +	}
> +
> +	tablet_notify_tool_update(base, time, new_tool);
> +}
> +
> +static void
>  tablet_flush(struct tablet_dispatch *tablet,
>  	     struct evdev_device *device,
>  	     uint32_t time)
>  {
> +	if (tablet_has_status(tablet, TABLET_TOOL_UPDATED))
> +		tablet_notify_tool(tablet, device, time);
> +
>  	if (tablet_has_status(tablet, TABLET_AXES_UPDATED)) {
>  		tablet_notify_axes(tablet, device, time);
>  		tablet_unset_status(tablet, TABLET_AXES_UPDATED);
> @@ -118,6 +206,12 @@ tablet_process(struct evdev_dispatch *dispatch,
>  	case EV_ABS:
>  		tablet_process_absolute(tablet, device, e, time);
>  		break;
> +	case EV_KEY:
> +		tablet_process_key(tablet, device, e, time);
> +		break;
> +	case EV_MSC:
> +		tablet_process_misc(tablet, device, e, time);
> +		break;
>  	case EV_SYN:
>  		tablet_flush(tablet, device, time);
>  		break;
> @@ -148,6 +242,7 @@ tablet_init(struct tablet_dispatch *tablet,
>  	tablet->base.interface = &tablet_interface;
>  	tablet->device = device;
>  	tablet->status = TABLET_NONE;
> +	tablet->current_tool_type = LIBINPUT_TOOL_NONE;
>  
>  	return 0;
>  }
> diff --git a/src/evdev-tablet.h b/src/evdev-tablet.h
> index d832c17..dd5988c 100644
> --- a/src/evdev-tablet.h
> +++ b/src/evdev-tablet.h
> @@ -29,7 +29,8 @@
>  
>  enum tablet_status {
>  	TABLET_NONE = 0,
> -	TABLET_AXES_UPDATED = 1 << 0
> +	TABLET_AXES_UPDATED = 1 << 0,
> +	TABLET_TOOL_UPDATED = 1 << 1
>  };

fwiw, for internal enums we don't have to worry about the trailing comma
- easier to just always append one.

>  
>  struct tablet_dispatch {
> @@ -39,6 +40,9 @@ struct tablet_dispatch {
>  	unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
>  	const struct input_absinfo *absinfo[LIBINPUT_TABLET_AXIS_CNT];
>  	double axes[LIBINPUT_TABLET_AXIS_CNT];
> +
> +	enum libinput_tool_type current_tool_type;
> +	uint32_t current_tool_serial;
>  };
>  
>  static inline enum libinput_tablet_axis
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index 83906f5..532ce29 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -54,6 +54,8 @@ struct libinput {
>  	size_t events_in;
>  	size_t events_out;
>  
> +	struct list tool_list;
> +
>  	const struct libinput_interface *interface;
>  	const struct libinput_interface_backend *interface_backend;
>  	void *user_data;
> @@ -208,6 +210,11 @@ 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);
> +
> +void
>  touch_notify_frame(struct libinput_device *device,
>  		   uint32_t time);
>  #endif /* LIBINPUT_PRIVATE_H */
> diff --git a/src/libinput.c b/src/libinput.c
> index def3e91..f484a5d 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -85,6 +85,7 @@ struct libinput_event_tablet {
>  	uint32_t time;
>  	double *axes;
>  	unsigned char changed_axes[NCHARS(LIBINPUT_TABLET_AXIS_CNT)];
> +	struct libinput_tool *tool;
>  };
>  
>  static void
> @@ -199,6 +200,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:
>  		break;
>  	}
>  
> @@ -226,6 +228,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:
>  		break;
>  	}
>  
> @@ -253,6 +256,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:
>  		break;
>  	}
>  
> @@ -279,6 +283,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:
>  		return (struct libinput_event_tablet *) event;
>  	}
>  
> @@ -305,6 +310,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:
>  		break;
>  	}
>  
> @@ -508,6 +514,12 @@ libinput_event_tablet_get_y_transformed(struct libinput_event_tablet *event,
>  					height);
>  }
>  
> +LIBINPUT_EXPORT struct libinput_tool *
> +libinput_event_tablet_get_tool(struct libinput_event_tablet *event)
> +{
> +	return event->tool;
> +}
> +
>  LIBINPUT_EXPORT uint32_t
>  libinput_event_tablet_get_time(struct libinput_event_tablet *event)
>  {
> @@ -604,6 +616,7 @@ libinput_init(struct libinput *libinput,
>  	libinput->user_data = user_data;
>  	list_init(&libinput->source_destroy_list);
>  	list_init(&libinput->seat_list);
> +	list_init(&libinput->tool_list);
>  
>  	if (libinput_timer_subsys_init(libinput) != 0) {
>  		free(libinput->events);
> @@ -673,6 +686,13 @@ libinput_event_destroy(struct libinput_event *event)
>  	if (event->device)
>  		libinput_device_unref(event->device);
>  
> +	else if (event->type == LIBINPUT_EVENT_TABLET_TOOL_UPDATE) {
> +		struct libinput_event_tablet *tevent =
> +			(struct libinput_event_tablet*)event;
> +
> +		libinput_tool_unref(tevent->tool);

this refcounting is not ideal. you have one ref in the libinput context, the
event itself doesn't have a explicit ref, it's tied to the event. If you
destroy it here, the tool disappears after each event if no client has a
ref. Which is what the API says, but we don't need to do this internally,
it's wasteful.

What we should do is unref the tool on libinput_destroy() only. That way we
keep it around once it has appeared somewhere and we get the behaviour we're
promising in the API without having to muck around any further. The downside
is - but we have to sort that out anyway: a tool with a serial of 0 cannot
be kept in the global tool list, instead each tablet object must have at
least one tool with a serial 0, so the code roughly looks like this:

struct libinput {
    struct list tool_list;
}

struct tablet_dispatch {
   struct libinput_tool *generic_tool; /* the serial 0 tool on this device */
}

update_tool(device, serial) {
    if (serial == 0)
        return device->default_tool;
    else if (libinput->tool_list has serial) {
        return that one
    else 
        allocate, add to tool_list
}

fix this patch up to unref in libinput_destroy() instead, not here. the
0-tool handling is better as a follow-up patch.

> +	}
> +
>  	free(event);
>  }
>  
> @@ -1159,6 +1179,27 @@ tablet_notify_axis(struct libinput_device *device,
>  			  &axis_event->base);
>  }
>  
> +void
> +tablet_notify_tool_update(struct libinput_device *device,
> +			  uint32_t time,
> +			  struct libinput_tool *tool)
> +{
> +	struct libinput_event_tablet *tool_update_event;
> +
> +	tool_update_event = zalloc(sizeof *tool_update_event);
> +	if (!tool_update_event)
> +		return;
> +
> +	*tool_update_event = (struct libinput_event_tablet) {
> +		.time = time,
> +		.tool = tool,
> +	};
> +
> +	post_device_event(device,
> +			  LIBINPUT_EVENT_TABLET_TOOL_UPDATE,
> +			  &tool_update_event->base);
> +}
> +
>  static void
>  libinput_post_event(struct libinput *libinput,
>  		    struct libinput_event *event)
> diff --git a/src/libinput.h b/src/libinput.h
> index c0c9fae..9615f1d 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -254,7 +254,12 @@ enum libinput_event_type {
>  	 */
>  	LIBINPUT_EVENT_TOUCH_FRAME,
>  
> -	LIBINPUT_EVENT_TABLET_AXIS = 600
> +	LIBINPUT_EVENT_TABLET_AXIS = 600,
> +	/**
> +	 * Signals that a device with the @ref LIBINPUT_DEVICE_CAP_TABLET
> +	 * capability has changed its tool.
> +	 */
> +	LIBINPUT_EVENT_TABLET_TOOL_UPDATE
>  };
>  
>  struct libinput;
> @@ -283,7 +288,8 @@ struct libinput_event_touch;
>   * @struct libinput_event_tablet
>   *
>   * Tablet event representing an axis update, button press, or tool update. Valid
> - * event types for this event are @ref LIBINPUT_EVENT_TABLET_AXIS.
> + * event types for this event are @ref LIBINPUT_EVENT_TABLET_AXIS, and
> + * @ref LIBINPUT_EVENT_TABLET_TOOL_UPDATE.
>   */
>  struct libinput_event_tablet;
>  
> @@ -891,6 +897,31 @@ libinput_event_tablet_get_x_transformed(struct libinput_event_tablet *event,
>  double
>  libinput_event_tablet_get_y_transformed(struct libinput_event_tablet *event,
>  					uint32_t height);
> +
> +/**
> + * @ingroup event_tablet
> + *
> + * Return the new tool in use 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 binded to the lifetime of the event, so the tool

"is bound"

with the changes above
Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

Cheers,
   Peter



> + * 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.
> + *
> + * @note It is an application bug to call this function for events other than
> + * @ref LIBINPUT_EVENT_TABLET_TOOL_UPDATE.
> + *
> + * @note On tablets where the serial number of tools is not reported, each tool
> + * cannot be guaranteed to be unique.
> + *
> + * @param event The libinput tablet event
> + * @return The new tool triggering this event
> + */
> +struct libinput_tool *
> +libinput_event_tablet_get_tool(struct libinput_event_tablet *event);
> +
>  /**
>   * @ingroup event_tablet
>   *
> -- 
> 1.8.5.5
 


More information about the wayland-devel mailing list