[RFC v2 libinput 2/2] buttonset: implement buttonset handling for Wacom tablet pads

[Peter Hutterer]

On Fri, Mar 27, 2015 at 02:52:15PM -0700, Jason Gerecke wrote:
> Did some practical testing of these patches and noticed two issues with the
> touchstrips:
> 
> On 3/17/2015 11:58 PM, Peter Hutterer wrote:
> >From: Benjamin Tissoires <benjamin.tissoires at gmail.com>
> >
> >Same approach as evdev-tablet (started as copy/paste), with axis and buttons
> >adjusted. Wacom's handling of pad devices requires a lot of non-obvious
> >handling, e.g. ABS_THROTTLE is the second ring, ABS_RX is the strip, etc.
> >
> >This is not generic buttonset code, if we start supporting other devices for
> >buttonsets we'll factor out a couple of the functions.
> >
> >The wheel and strip events are a bit of a problem: Wacom sends a 0 event on the
> >axis when the finger is released. We can detect this if there is an ABS_MISC 0
> >present in the same event and suppress it.  Won't work if any finger is down
> >on any other wheel, strip or button but that's a kernel bug we'll fix once we
> >figure out how.
> >
> >Signed-off-by: Benjamin Tissoires <benjamin.tissoires at redhat.com>
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >  src/Makefile.am             |   2 +
> >  src/evdev-buttonset-wacom.c | 533 ++++++++++++++++++++++++++++++++++++++++++++
> >  src/evdev-buttonset-wacom.h |  63 ++++++
> >  src/evdev.c                 |  18 +-
> >  src/evdev.h                 |   3 +
> >  src/libinput-private.h      |  16 ++
> >  src/libinput.c              |  65 ++++++
> >  7 files changed, 691 insertions(+), 9 deletions(-)
> >  create mode 100644 src/evdev-buttonset-wacom.c
> >  create mode 100644 src/evdev-buttonset-wacom.h
> >
> >[...]
> >
> >+static inline double
> >+normalize_strip(const struct input_absinfo *absinfo)
> >+{
> >+	/* strip axes don't use a proper value, they just shift the bit left
> >+	 * for each position. 0 isn't a real value either, it's only sent on
> >+	 * finger release */
> >+	double min = 1,
> 
> Min should be set to 0 (i.e., log2(1)) here.

fixed, thanks.
 
> >+	       max = log2(absinfo->maximum);
> >+	double range = max - min;
> >+	double value = (log2(absinfo->value) - min) / range;
> >+
> >+	return value;
> >+}
> >+
> >+/* Detect ring wraparound, current and old are normalized to [0, 1[ */
> >+static inline double
> >+guess_ring_delta(double current, double old)
> >+{
> >+	double d1, d2, d3;
> >+
> >+	d1 = current - old;
> >+	d2 = (current + 1) - old;
> >+	d3 = current - (old + 1);
> >+
> >+	if (fabs(d2) < fabs(d1))
> >+		d1 = d2;
> >+
> >+	if (fabs(d3) < fabs(d1))
> >+		d1 = d3;
> >+
> >+	return d1;
> >+}
> >+
> >+static void
> >+buttonset_check_notify_axes(struct buttonset_dispatch *buttonset,
> >+			    struct evdev_device *device,
> >+			    uint32_t time)
> >+{
> >+	struct libinput_device *base = &device->base;
> >+	bool axis_update_needed = false;
> >+	double deltas[LIBINPUT_BUTTONSET_MAX_NUM_AXES] = {0};
> >+	double deltas_discrete[LIBINPUT_BUTTONSET_MAX_NUM_AXES] = {0};
> >+	unsigned int a;
> >+	unsigned int code;
> >+
> >+	for (a = 0; a <= buttonset->naxes; a++) {
> >+		const struct input_absinfo *absinfo;
> >+
> >+		if (!bit_is_set(buttonset->changed_axes, a))
> >+			continue;
> >+
> >+		code = buttonset->axis_map[a];
> >+		assert(code != 0);
> >+		absinfo = libevdev_get_abs_info(device->evdev, code);
> >+		assert(absinfo);
> >+
> >+		switch (buttonset->types[a]) {
> >+		case LIBINPUT_BUTTONSET_AXIS_RING:
> >+			buttonset->axes[a] = normalize_ring(absinfo);
> >+			deltas[a] = guess_ring_delta(buttonset->axes[a],
> >+						     buttonset->axes_prev[a]);
> >+			deltas_discrete[a] = unnormalize_ring_value(absinfo,
> >+								    deltas[a]);
> >+			break;
> >+		case LIBINPUT_BUTTONSET_AXIS_STRIP:
> >+			buttonset->axes[a] = normalize_strip(absinfo);
> >+			deltas[a] = buttonset->axes[a] - buttonset->axes_prev[a];
> >+			break;
> 
> When a touch goes up, normalize_strip will return a normalized value of
> negative infinity. You should add suppression logic like for the ABS_MISC
> case since it isn't a real value.

added, thanks.

Cheers,
   Peter

> 
> >+		default:
> >+			log_bug_libinput(device->base.seat->libinput,
> >+					 "Invalid axis update: %u\n", a);
> >+			break;
> >+		}
> >+
> >+		if (buttonset->have_abs_misc_terminator) {
> >+			/* Suppress the reset to 0 on finger up. See the
> >+			   comment in buttonset_process_absolute */
> >+			if (libevdev_get_event_value(device->evdev,
> >+						     EV_ABS,
> >+						     ABS_MISC) == 0) {
> >+				clear_bit(buttonset->changed_axes, a);
> >+				buttonset->axes[a] = buttonset->axes_prev[a];
> >+				continue;
> >+			/* on finger down, reset the delta to 0 */
> >+			} else {
> >+				deltas[a] = 0;
> >+				deltas_discrete[a] = 0;
> >+			}
> >+		}
> >+
> >+		axis_update_needed = true;
> >+	}
> >+
> >+	if (axis_update_needed)
> >+		buttonset_notify_axis(base,
> >+				      time,
> >+				      LIBINPUT_BUTTONSET_AXIS_SOURCE_UNKNOWN,
> >+				      buttonset->changed_axes,
> >+				      buttonset->axes,
> >+				      deltas,
> >+				      deltas_discrete);
> >+
> >+	memset(buttonset->changed_axes, 0, sizeof(buttonset->changed_axes));
> >+	memcpy(buttonset->axes_prev,
> >+	       buttonset->axes,
> >+	       sizeof(buttonset->axes_prev));
> >+	buttonset->have_abs_misc_terminator = false;
> >+}
> >+
> >[...]
> >
> 
> -- 
> Jason
> ---
> Now instead of four in the eights place /
> you’ve got three, ‘Cause you added one /
> (That is to say, eight) to the two, /
> But you can’t take seven from three, /
> So you look at the sixty-fours....